| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-21 | |||
| 20:45:03 | mriedem | we could create the attachment for the new volume in the api, but we don't have a bdm for that | |
| 20:45:18 | mriedem | so not sure how we get that in the compute, unless we lookup the new volume details and get it's attachment list | |
| 20:45:21 | coffee_cat | yeah, BDM is problematic :/ | |
| 20:46:25 | mriedem | maybe he just meant create the new attachment in the api so we can fail fast | |
| 20:46:39 | mriedem | but, | |
| 20:46:42 | coffee_cat | probably as that was a topic many times | |
| 20:46:59 | mriedem | we'd only do that if we knew the old attached volume was created with the new flow i htink | |
| 20:47:01 | mriedem | *think | |
| 20:47:05 | mriedem | i'm not sure if that matters or not | |
| 20:47:08 | mriedem | but it probably does | |
| 20:47:33 | coffee_cat | so the volume is always created the same way | |
| 20:47:58 | mriedem | this is all new flow stuff | |
| 20:47:59 | mriedem | "In this section, we talk about what happens should the volume being swapped have the attachment_id present in the BDM, and as such we follow the new flow." | |
| 20:48:25 | mriedem | yeah, so if the bdm.attachment_id is set, we key off that for all of the swap volume goodies outlined in the spec | |
| 20:48:31 | mriedem | like creating a new attachment for the new volume, etc | |
| 20:48:36 | coffee_cat | and for the new volume we can create that with the new flow | |
| 20:49:01 | coffee_cat | if the API is available | |
| 20:49:11 | mriedem | well i think the point is, you can't swap from a volume attachd the old way, to a volume attached the new way | |
| 20:49:28 | mriedem | you have to swap from attached new way to attached new way | |
| 20:49:30 | mriedem | if that makes sense | |
| 20:49:42 | mriedem | i don't know if technically it makes a difference on the cinder side | |
| 20:49:46 | coffee_cat | I don't really see why not, but as we go by attachment_id in the BDM anyhow it pretty much doesn't matter | |
| 20:49:49 | sdague | mriedem: it would be less terrible if you used the fixture that actually captures the log stream | |
| 20:50:22 | smcginnis | mriedem: Should be OK on the Cinder side as long as each volume has its attachment info. | |
| 20:50:22 | mriedem | coffee_cat: the reason john and i said to do it this way in the spec is so nothing else in the code does anything using the new apis until we've plumbed in support for attaching volumes the new way | |
| 20:50:25 | mriedem | since everything keys off that | |
| 20:50:35 | coffee_cat | in the sense that we only detach --> attach with the new flow if the BDM.attachment_id is set | |
| 20:50:38 | smcginnis | mriedem: ie, we don't do some silly DB swap of some data to "make it easier" | |
| 20:51:04 | mriedem | coffee_cat: i think i'm agreeing with you | |
| 20:51:47 | mriedem | the thing i don't like about https://specs.openstack.org/openstack/nova-specs/specs/pike/approved/cinder-new-attach-apis.html#swap-volume and creating the new attachment for the new volume in the API is, we'd have to do a service version check on that b/c we wouldn't know if the compute we're going to cast to is new enough to understand what we're doing | |
| 20:51:50 | coffee_cat | mriedem: ok, if we want to do this in the API, where we don't have the BDM that's another thing | |
| 20:52:04 | mriedem | i don't think we want to do it in the api | |
| 20:52:15 | mriedem | since that logic relies on if the compute is new enough i think | |
| 20:52:27 | mriedem | if we're casting to an ocata compute, it's not going to have any of this new bdm.attachment_id code | |
| 20:52:32 | coffee_cat | although there we need to check whether the new Cinder API is available, etc. | |
| 20:52:50 | mriedem | well, that's why we key off bdm.attachment_id, | |
| 20:52:53 | mriedem | if that is set, we're using new api | |
| 20:53:05 | mriedem | that's why new style attach comes at the very end | |
| 20:53:13 | coffee_cat | mriedem: yeah, on the compute it's all fine and I think we're on the same page if we keep things there | |
| 20:53:13 | mriedem | since everything else keys off that | |
| 20:53:26 | mriedem | coffee_cat: ok i can amend the spec for that | |
| 20:53:31 | mriedem | when i add in the stuff we talked about this morning | |
| 20:53:39 | mriedem | about begin_detaching | |
| 20:53:51 | coffee_cat | mriedem: sounds good, thank you | |
| 21:12:13 | mriedem | maybe john had us creating the new attachment in the api in place of calling reserve on the new volume | |
| 21:13:03 | mriedem | but i thought with the new flow, we still called reserve when doing an attach? | |
| 21:13:59 | mriedem | coffee_cat: do you remember? ^ | |
| 21:14:11 | mriedem | the spec doesn't talk about reserve | |
| 21:15:09 | mriedem | hmm, the cinder spec talks about it http://specs.openstack.org/openstack/cinder-specs/specs/ocata/add-new-attach-apis.html#proposed-change | |
| 21:15:33 | mriedem | "If the connector is not specified this call will only complete the volume reservation step and create an empty volume attachment. This can be used by Nova to call it from the API and start the volume attaching process by checking that the volume is available and creates the empty attachment." | |
| 21:16:19 | mriedem | and in the nova spec | |
| 21:16:20 | mriedem | https://specs.openstack.org/openstack/nova-specs/specs/pike/approved/cinder-new-attach-apis.html#proposed-change | |
| 21:16:29 | mriedem | "In addition we should only use the new API when all of the nova-compute nodes have been upgraded." | |
| 21:17:05 | mriedem | so if we only create a new attachment when cinder 3.27 is available and all computes are upgraded, then during swap_volume we can create the new attachment for the new volume based on bdm.attachment_id being set for the old volume | |
| 21:17:31 | mriedem | we could pass the new attachment_id over rpc to swap_volume on the compute | |
| 21:18:32 | coffee_cat | mriedem: we don't call reserve with the new flow | |
| 21:18:53 | mriedem | coffee_cat: yeah ok so then we need to create the new attachment in the api | |
| 21:18:58 | coffee_cat | mriedem: attachment_create without connector acts as reserve in the sense of putting the volume in 'reserved' state | |
| 21:19:05 | mriedem | and pass the attachment id down to the compute over rpc, or the compute can look it up | |
| 21:19:52 | coffee_cat | I guess passing it to the compute should be fine if we don't want to call out to Cinder that much | |
| 21:20:47 | mriedem | the compute could lookup the attachments for the new volume... | |
| 21:21:02 | mriedem | since the cinder api isn't documented, i don't know what it looks like | |
| 21:21:38 | coffee_cat | mriedem: also we need to figure out on the compute too how to finalize the attachment process | |
| 21:21:58 | coffee_cat | we might do that by using receiving an attachment_id from the API as a trigger | |
| 21:22:38 | coffee_cat | or list the attachments with the call from the new flow too I guess | |
| 21:23:22 | mriedem | we update the attachment with the connector | |
| 21:24:38 | coffee_cat | I meant that we need to know on the compute that the API reserved the volume with the new flow | |
| 21:24:50 | coffee_cat | and we need to call attachment_update at some point | |
| 21:25:05 | coffee_cat | with the connector | |
| 21:25:34 | mriedem | if bdm.attachment_id is set, we know the api reserved the new volume with the new flow | |
| 21:25:47 | mriedem | so i think https://review.openstack.org/#/c/456971/ gets more complicated than just updating the compute code | |
| 21:25:50 | mriedem | it should also be the api code | |
| 21:25:54 | mriedem | and the rpc api changes | |
| 21:26:09 | mriedem | unless we just want to lookup the new attachment via rest api | |
| 21:26:17 | mriedem | i just don't know if we have that support yet in cinderclient | |
| 21:26:53 | mriedem | i could help work on this if you want | |
| 21:27:02 | mriedem | i just can't do it today | |
| 21:28:02 | mriedem | it would be great if you guys could get the v3.27 APIs documented in https://developer.openstack.org/api-ref/block-storage/v3/ | |
| 21:28:04 | mriedem | you == all of cinder | |
| 21:28:31 | mriedem | and by "guys" i mean guys and gals and those that don't identify as guys or gals, of course | |
| 21:31:12 | coffee_cat | mriedem: got it :) | |
| 21:31:32 | mriedem | it looks like from the compute we could list attachments and filter on instance_uuid | |
| 21:31:53 | coffee_cat | and yeah, if the bdm.attachment_id is set then all computes should be on the high enough version, etc. | |
| 21:32:23 | coffee_cat | we should definitely be able to do that if we want to skip the rpc changes, which I'm totally fine with | |
| 21:32:28 | mriedem | we could get attachments via cinderclient for a given volume and instance | |
| 21:32:33 | mriedem | using filters it looks like | |
| 21:32:40 | jgriffith | mriedem correct | |
| 21:33:01 | mriedem | if we got more than one attachment back...idk | |
| 21:33:10 | mriedem | we shouldn't today since we don't support multiattach | |
| 21:33:12 | mriedem | but it gets weird | |
| 21:33:21 | mriedem | that's probably why it's just better to pass the new attachment id over rpc | |
| 21:33:22 | mriedem | to the compute | |
| 21:33:22 | jgriffith | mriedem right, for today that’s our safety net | |
| 21:33:26 | mriedem | rather than hae the compute guess | |
| 21:33:30 | jgriffith | BUT, there’s a get-attachment | |
| 21:33:35 | coffee_cat | mriedem: although for live migrate etc we should be able to have multiple attachments for the same volume | |
| 21:33:51 | coffee_cat | which should not make a mess for swap volume, but still | |
| 21:33:58 | mriedem | i think passing over rpc is safest | |
| 21:34:05 | mriedem | then compute doesn't need to decide | |
| 21:34:23 | mriedem | again, i can help with this later, but i'm getting in trouble for hiding in the office when it's a nice day out | |