17:00:06 <ildikov> #startmeeting cinder-nova-api-changes
17:00:06 <openstack> Meeting started Mon Nov 28 17:00:06 2016 UTC and is due to finish in 60 minutes.  The chair is ildikov. Information about MeetBot at http://wiki.debian.org/MeetBot.
17:00:07 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:00:10 <openstack> The meeting name has been set to 'cinder_nova_api_changes'
17:00:19 <ildikov> scottda ildikov DuncanT ameade cFouts johnthetubaguy jaypipes takashin alaski e0ne jgriffith tbarron andrearosa hemna erlon mriedem gouthamr ebalduf patrickeast smcginnis diablo_rojo gsilvis  xyang1 raj_singh lyarwood
17:00:35 <johnthetubaguy> o/
17:01:04 <ildikov> johnthetubaguy: hi :)
17:01:33 * ildikov wonders whether we will see any US people around after the long turkey weekend... :)
17:02:24 * johnthetubaguy makes clucking noises and waggles his elbows (in his head)
17:02:59 <ildikov> LOL :)
17:03:27 <ildikov> mriedem: did you have a chance to check on the reviews last week before the long w/e?
17:03:41 <mriedem> nope
17:04:07 <ildikov> mriedem: what about doing it now?
17:04:47 <ildikov> mriedem: the remove check_attach is a smaller one
17:04:57 <johnthetubaguy> so I don't think I have had a proper look at the cinder spec either, I should do that
17:05:01 <ildikov> mriedem: and would be a nice cleanup to get it done IMHO
17:05:35 <ildikov> johnthetubaguy: that's a fairly short one updated by me recently, so we can chat about that too
17:07:30 <ildikov> johnthetubaguy: regarding the Cinder spec we have a call there with the functionality of reserving a volume and creating an empty attachment and another call that can create an attachment with all the info or update an existing empty attachment
17:08:18 <ildikov> johnthetubaguy: I think the code might check currently whether the volume is reserved or not, but I would need to double check that to be sure
17:09:33 <jgriffith> o/
17:09:53 <jgriffith> johnthetubaguy: the clucking noises drew me in
17:10:16 <johnthetubaguy> :)
17:10:51 * ildikov thanks johnthetubaguy for the effective noises :)
17:11:01 <jgriffith> johnthetubaguy: the code will in fact check to see if it's already reserved
17:11:12 <jgriffith> johnthetubaguy: and if so raise an exception
17:11:28 <jgriffith> johnthetubaguy: ie if you try and do a simultaneous reserve
17:12:18 <johnthetubaguy> that sounds like it all makes sense
17:12:22 <johnthetubaguy> just got my head in the spec
17:13:31 <ildikov> johnthetubaguy: I added a few notes to the attachment-reserve based on the last week's discussion and also jaypipes had a comment on that one earlier regarding the name vs functionality
17:13:51 <ildikov> johnthetubaguy: jgriffith: if we can agree on the functionality that would be pretty great
17:14:21 <johnthetubaguy> I thought jay's comments were around shelve, which is a bit orthogonal, but maybe I forgot some of them.
17:14:24 <ildikov> johnthetubaguy: jgriffith: we can then finalize the names and get some code merged
17:14:40 <ildikov> johnthetubaguy: he had a few comments on the Cinder spec too, I meant those
17:14:46 <jgriffith> johnthetubaguy: he also had some comments about if/why we would even want the reserve call
17:15:35 <johnthetubaguy> the nova spec tried to cover why its needed, mostly around a better user experience
17:15:36 <ildikov> I think his point was that we reserve the volume in an attachment-reserve call, which is a bit of a mismatch
17:15:56 <jgriffith> ildikov: yeah, that too :)
17:16:42 <ildikov> jgriffith: just sayin' :)
17:16:54 <jgriffith> ildikov: hey.. that's my "catch phrase" :)
17:17:16 <johnthetubaguy> well depends if you consider the volume attached to both a host and an instance I guess
17:17:38 <jgriffith> I personally like the name just fine, it reserves an attachment for the volume
17:17:52 <ildikov> jgriffith: oooops :)
17:17:56 <johnthetubaguy> its not clear from the spec how this relates to REST like resources, just thinking about if we are following the API wg guidelines
17:17:57 <jgriffith> we have sort of discussed this as a group at nauseam
17:19:17 <johnthetubaguy> right, I would like to get it written down in a way we can all say, yes thats what we agree
17:19:36 <jgriffith> johnthetubaguy: roger
17:19:46 <ildikov> johnthetubaguy: do we know already what is what we all agree?
17:20:42 <johnthetubaguy> writing it down often helps
17:20:44 <ildikov> johnthetubaguy: I'm more than happy to write it down, or draw it or create a statue for it or whatever that keeps people satisfied...
17:21:10 <johnthetubaguy> so the Nova spec tried to cover all the details of both sides
17:21:33 <ildikov> johnthetubaguy: is your spec matching with what is written today in the Cinder spec?
17:21:57 <ildikov> johnthetubaguy: more regarding functionality than the names just to try to start with smth solid
17:22:00 <johnthetubaguy> honestly, the only difference seems to be around slightly different structure of the REST API calls, which I care much less about than the functionality
17:22:24 <hemna> hey
17:22:50 <ildikov> johnthetubaguy: I think last time we said something like don't always create new attachments, etc.
17:23:43 <ildikov> johnthetubaguy: I think at least the Cinder side currently handles things more like that model, but jgriffith can correct me based on the code
17:23:48 <ildikov> hemna: hi
17:24:04 <hemna> still doing my morning coffee here
17:24:41 <hemna> what are we arguing over today? :P
17:24:46 <ildikov> hemna: we are trying to get there to agree on the API functionality regarding Cinder and have our heads around the specs on both sides
17:24:51 <jgriffith> ildikov: I'm not completely sure, I'll have to look at the deltas johnthetubaguy 's is pointing out
17:24:53 <johnthetubaguy> ildikov: rather than create a new attachment, just update the existing one with a empty connector, etc, yeah, that bit is missing
17:25:12 <jgriffith> johnthetubaguy: oh...
17:25:26 <jgriffith> johnthetubaguy: so yeah; the attachment-create fulfills that
17:25:26 <ildikov> jgriffith: I think it's more what we discussed last week
17:25:32 <jgriffith> ildikov: +1
17:25:33 <ildikov> jgriffith: I linked the meeting log to the Cinder spec
17:26:23 <ildikov> jgriffith: but the create updates only if the volume is in "reserved" state right?
17:26:24 <jgriffith> johnthetubaguy: ildikov I guess I'm open to changing the naming conventions there; but I personally liked the reserve/create semantics and using an existing attachment object if provided
17:26:32 <johnthetubaguy> so if create can be called with only the volume_id and instance_uuid, and returns an attachment_id, it seems like we don't need reserved I guess?
17:26:59 <jgriffith> johnthetubaguy: right, but we needed it for things like BFV where you don't have a connector and you need the volume
17:27:13 <johnthetubaguy> I dunno, I just think a regular CRUD REST API would be simpler
17:27:16 <jgriffith> johnthetubaguy: otherwise we end up back at V1 :)
17:27:25 <hemna> jgriffith, +1
17:27:25 <jgriffith> johnthetubaguy: I'm open, can you ellaborate?
17:27:34 <ildikov> BTW if in any point in time you feel the need to talk as opposed to write I can create a Hangouts link
17:28:02 <johnthetubaguy> so where is /attachments going? is its volume/{uuid}/attachments ?
17:28:23 <johnthetubaguy> this might be easier in text, for the REST thing
17:28:38 <jgriffith> johnthetubaguy: it goes to attachments
17:28:56 <ildikov> johnthetubaguy: was just a side note, you know, just in case :)
17:28:59 <johnthetubaguy> I mean the URL, what does it look like?
17:29:00 <jgriffith> johnthetubaguy: so the "attachment_create" is sort of different if you don't provide an Attachment UUID it creates one for you
17:29:39 <johnthetubaguy> so you are POST ing to some URL I assume, what is the URL?
17:31:28 <jgriffith> johnthetubaguy: sorry...
17:31:45 <jgriffith> johnthetubaguy: so yeah; /attachments/attachment
17:31:55 <johnthetubaguy> I am basically trying to check if the cinder spec/design conforms with what is here: https://specs.openstack.org/openstack/api-wg/guidelines/uri.html#general-advice-on-uri-design
17:32:03 <jgriffith> that attachment body has either a volume.uuid or a attachment.uuid
17:32:34 <johnthetubaguy> and this bit: https://specs.openstack.org/openstack/api-wg/guidelines/http.html#http-methods
17:32:39 <jgriffith> johnthetubaguy: on the Get and Delete yes... but the attachment-create call is kinda goofy
17:33:02 <jgriffith> johnthetubaguy: but it does conform to that POST  definition
17:33:29 <jgriffith> johnthetubaguy: ildikov if it makes things better/easier I'm happy to break that into to calls; the create and an update
17:34:12 <jgriffith> johnthetubaguy: ildikov in which case I think it would solve the confusion and potential issues around the API guidelines (confusion==my-code-being-confusing)
17:34:17 <ildikov> jgriffith: in addition to reserve or we drop reserve in that case?
17:34:32 <jgriffith> ildikov: we need to keep reserve based on discussions we'v had in the past
17:34:39 <johnthetubaguy> PUT attachement/{uuid} might do the create or update you want
17:34:58 <johnthetubaguy> ah, but not really, you would have to pick the uuid, which is nuts
17:35:11 <jgriffith> johnthetubaguy: yeah; the problem is the way I do it currently there's a hack to make it so you don't need the attachment UUID
17:35:28 <jgriffith> johnthetubaguy: so yes, I'm saying I'll rework it to do what you want
17:35:45 <jgriffith> johnthetubaguy: or, what the guides says I should do to make it more correct
17:36:09 <johnthetubaguy> yeah, I think following that pattern would be good
17:36:14 <ildikov> jgriffith: less arguing on naming at least
17:36:31 <hemna> and for bare metal attaches, or non nova/ironic attaches the caller just creates a UUID to pass in and they have to track it?
17:36:37 <johnthetubaguy> honestly the POST method to create and the update are probably calling the same code largely, so its probably not a big deal
17:37:43 <johnthetubaguy> so I guess I was expecting to list attachments by doing GET volume/{uuid}/attachments or /attachments?volume_uuid={uuid}&host_uuid={host_uuid} etc
17:37:44 <ildikov> hemna: I guess if create has the connector than it can do the whole magic
17:38:03 <ildikov> hemna: and they get back the attachment_id as in the current version
17:38:06 <jgriffith> hemna: no, there will still be an option to do that
17:38:11 <hemna> ok
17:38:18 <jgriffith> ildikov: +1
17:38:42 <johnthetubaguy> right, the create call, whatever it is, should be able to do the full operation in one shot
17:38:57 <hemna> for example, there are other projects that are now starting to look at doing local cinder volume attaches
17:39:00 <hemna> https://review.openstack.org/#/c/385880/
17:39:01 <hemna> fyi
17:39:31 <ildikov> johnthetubaguy: do we still want to get an attachment_id back from the reserve call?
17:39:40 <ildikov> johnthetubaguy: I mean Nova :)
17:39:46 <jgriffith> ildikov: johnthetubaguy I'l answer *yes*
17:39:55 <jgriffith> ildikov: johnthetubaguy it's for tracking around in Cinder
17:40:24 <ildikov> jgriffith: just checkin' :)
17:40:27 <johnthetubaguy> I am find with create-attachment taking only instance-uuid and volume-uuid, rather than a specific reserve call, but it seems like all the same thing
17:40:27 <jgriffith> ildikov: johnthetubaguy removes the need for trying to guess in a multi-attach scenario and gives us an authoritative reference
17:41:24 <johnthetubaguy> right, Nova still need to know about multi-attach, but its just a property that changes how we attach the volume, rather than something we need to code up in the API
17:41:25 <ildikov> jgriffith: right, seems more bullet proof
17:41:54 <jgriffith> johnthetubaguy: we need the reserve for special cases.  So just to be clear; and I'll work with ildikov to update the Cinder spec
17:42:00 <jgriffith> johnthetubaguy: ildikov we'll have:
17:42:29 <jgriffith> 1. attachment_reserve (creates empty attachment and returns the UUID of attachment record)
17:43:04 <jgriffith> 2. attachment_update (takes the connector and finalizes things when you're ready for it and works off of an attachment UUID only)
17:43:26 <jgriffith> 3. Something else for folks that want an ALL in One create-attachment, and finalize
17:43:54 <ildikov> if we are aiming for CRUD, wouldn't it be reserve and create?
17:44:13 <jgriffith> whether item 1 or item 3 should be CREATE or both should be is another question
17:44:18 <johnthetubaguy> seems like create could do the all in one operation
17:44:21 <ildikov> and update for cases, when we need to update the connector as Nova has some weird use cases to keep the whole world happy?
17:44:47 <ildikov> and others just don't use update as it does not make any sense for them I guess
17:44:53 <jgriffith> johnthetubaguy: well it does currently, but it was pointed out that didn't follow the CRUD guidelines :)
17:45:06 <johnthetubaguy> its more that update wasn't separate for me
17:45:16 <johnthetubaguy> create can setup *everything* thats totally fine
17:45:23 <jgriffith> johnthetubaguy: ahh!
17:45:33 <jgriffith> johnthetubaguy: ok, great, we have a plan then
17:45:38 <ildikov> johnthetubaguy: +1
17:45:40 <hemna> that sounds good
17:45:55 <jgriffith> a reserve, create and update
17:46:05 <jgriffith> I'll update that happily
17:46:32 <johnthetubaguy> so don't shoot me, but...
17:46:39 <jgriffith> haha
17:46:48 * hemna locks and loads....
17:46:48 <johnthetubaguy> reserve, thats just a create with connector=None?
17:47:14 <jgriffith> johnthetubaguy: yes
17:47:19 <jgriffith> johnthetubaguy: that's the problem
17:47:33 <johnthetubaguy> so we can just have create and update then (along with get and delete)
17:47:39 <ildikov> johnthetubaguy: I would say with no connector and no attachment_id
17:47:40 <jgriffith> johnthetubaguy: works for me
17:47:58 <johnthetubaguy> ildikov: create never takes attachment_id right? thats update
17:48:10 <ildikov> so create will "lock" the volume in this case?
17:48:16 <johnthetubaguy> jgriffith: cool
17:48:33 <johnthetubaguy> ildikov: in every case where you specify and instance_uuid on create
17:48:51 <johnthetubaguy> ildikov: just sometimes it does everything
17:48:59 <jgriffith> so we have attachment_create; and that can be called as a reserve, or as a "do everything for me" and returns an attachment UUID
17:49:06 <johnthetubaguy> yeah
17:49:15 <ildikov> johnthetubaguy: I'm just asking from Nova perspective as I guess we would call create without connector from the API and update from the compute
17:49:19 <jgriffith> we can then have an update for those that were just a reserve to provide a connector and finish things off
17:49:30 <johnthetubaguy> cool
17:49:43 <hemna> +1
17:49:52 <ildikov> jgriffith: that's how I meant, cool :)
17:49:53 <johnthetubaguy> also, when we reschedule a build, we can just update the connector to None when we detach?
17:49:55 <jgriffith> ildikov: does that sound good to you?
17:49:56 <jgriffith> ok
17:50:07 <jgriffith> alright, let's knock this thing out this week!!!
17:50:07 <ildikov> and that's why I asked earlier whether we really need a "reserve" :)
17:50:24 <jgriffith> ildikov: yea, we need it for bfv and some other cases
17:50:28 <jgriffith> ha
17:50:38 <jgriffith> but now we just fold it into create
17:50:39 <hemna> so what is Nova going to do in the API ?  call create w/o an ID and that means reserve?
17:50:47 <johnthetubaguy> yeah
17:50:50 <jgriffith> I'm too hung up on what it's doing as opposed to the naming
17:50:51 <hemna> ok cool
17:51:31 <jgriffith> hemna: johnthetubaguy I think you mean "create without a connector"
17:51:45 <johnthetubaguy> yup
17:51:46 <jgriffith> which internally results in a reserve
17:51:52 <johnthetubaguy> has instance_uuid
17:52:00 <johnthetubaguy> no connector, no host_uuid
17:52:01 <ildikov> jgriffith: what is the next dessert on the list? :)
17:52:13 <jgriffith> johnthetubaguy: yes
17:52:17 <jgriffith> ildikov: LOL
17:52:19 <jgriffith> ApplePie
17:52:25 <hemna> ok cool yah
17:52:28 <ildikov> jgriffith: just t not deal with the names but what we want it to do
17:52:55 <ildikov> jgriffith: with vanilla ice cream; it works :)
17:53:08 <hemna> French Vanilla
17:53:57 <ildikov> hemna: only the best, I'm not negotiating on important things like this :)
17:54:09 <johnthetubaguy> so update, there is a bit about changing the host from last week
17:54:36 <johnthetubaguy> I think we just call with a None connector, to say we detached and are starting to trying somewhere new
17:54:43 <johnthetubaguy> and a None host_uuid
17:55:04 <ildikov> johnthetubaguy: changing the host as rebuild or evacuate?
17:55:19 <johnthetubaguy> (the alternative being create a new attachment, and delete the old one)
17:55:37 <jgriffith> johnthetubaguy: I'd prefer delete/create if it works for you
17:55:43 <johnthetubaguy> ildikov: this is specific to reschedule after an error during the initial build
17:55:50 <jgriffith> johnthetubaguy: but we could try and do something clever and fold it into the update
17:56:01 <ildikov> johnthetubaguy: yeap, that would've been my third guess, tnx
17:56:08 <johnthetubaguy> jgriffith: I think I am fine leaving that for a v2
17:56:16 <jgriffith> johnthetubaguy: I guess there's no reason not to allow update to take a connector and resetting things
17:56:41 <jgriffith> johnthetubaguy: I'll look at it when I get to that code and see if it's trivial and clear without overloading the heck out of the api call
17:56:47 <johnthetubaguy> personally I find delete/create simpler to reason about, but I think other nova folks preferred the update to do the reset
17:57:02 <johnthetubaguy> jgriffith: sounds cool
17:57:03 <jgriffith> johnthetubaguy: yes, I definitely prefer delete/create
17:57:20 <jgriffith> but certainly willing to look at update if it makes everybody happy
17:57:27 <johnthetubaguy> I think form our BDM point of view, its nice to keep the same attachment id across the "fiddling", but I know there are cases where we can't
17:57:59 <johnthetubaguy> it feels like an optimisation we can discuss later myself
17:58:00 <johnthetubaguy> yeah
17:58:03 <ildikov> johnthetubaguy: from cleanup perspective update sounds fine, but I guess at this point it's purely Nova's choice how to use the Cinder API
17:58:46 <ildikov> I mean if we have create and update separately then we can either create a new attachment or just update the existing
17:59:52 <ildikov> does Nova return any id to the user from the API or it's fully internal?
17:59:58 <ildikov> johnthetubaguy: ^
18:00:51 <johnthetubaguy> not sure actually
18:00:58 <johnthetubaguy> I think there is something
18:01:38 <johnthetubaguy> http://developer.openstack.org/api-ref/compute/?expanded=list-volume-attachments-for-an-instance-detail
18:01:48 <johnthetubaguy> so you will love this... our attachment id is the volume id
18:01:58 <jgriffith> doh
18:02:14 <johnthetubaguy> luckily is scoped per server, so its not a big deal
18:03:15 <johnthetubaguy> so I think we made progress there
18:03:18 <johnthetubaguy> but we are out of time
18:03:30 <johnthetubaguy> and I need to head off to drop my car in for a new timing belt
18:03:35 <ildikov> johnthetubaguy: if it's the API response then I guess it's slightly ugly to update the attachment_id on a retry
18:03:56 <johnthetubaguy> ildikov: its OK, we use the volume uuid
18:04:17 <johnthetubaguy> ildikov: we don't currently store the attachment-id anywhere, mostly as it may not exist right now
18:04:18 <ildikov> johnthetubaguy: but that's not the Cinder API's problem, so I approve you go and take care of the car
18:04:40 <johnthetubaguy> I think we are good with the crud on attachments above
18:04:40 <ildikov> johnthetubaguy: well, we ask Cinder for it every time it's used for detach
18:05:01 <johnthetubaguy> right, it would be nice if we new which one we are detaching before that
18:05:50 <ildikov> johnthetubaguy: the CRUD Cinder API will give the chance the Nova to use it in the right way I think
18:06:13 <ildikov> johnthetubaguy: I'll try to get myself friends with the thought of touching the BDM...
18:06:29 <jgriffith> ildikov: johnthetubaguy I have a patch that stuffs it in the bdm already
18:06:38 <jgriffith> would assume we'd continue with that
18:06:49 <jgriffith> easily solvable :)
18:07:06 <johnthetubaguy> it should be fine
18:07:14 <ildikov> jgriffith: you're my hero! :)
18:07:23 <ildikov> jgriffith: I'll check it this week
18:07:25 <johnthetubaguy> there is the migration plan for upgrades, where we update attachments via cinder manage, but thats for another day
18:07:29 <jgriffith> LOL
18:07:43 <ildikov> johnthetubaguy: you're the one who has to run :)
18:08:08 <johnthetubaguy> true...
18:08:19 <johnthetubaguy> I should go before I get too hungry really
18:08:44 <ildikov> johnthetubaguy: I didn't want to chase you away really
18:09:19 <ildikov> ok, let's do the spec and code updates this week
18:09:31 <ildikov> I think we are in an agreement so far
18:10:07 <hemna> yup
18:10:13 <hemna> lets rock this one out finally
18:10:27 <ildikov> you can correct me if I got it wrong but I would prefer not to :)
18:10:37 <ildikov> hemna: +1 :)
18:10:52 <ildikov> anything else from anyone for today?
18:12:04 <ildikov> all right, thank you all
18:12:15 <ildikov> have a nice rest of the day!
18:12:21 <ildikov> #endmeeting