16:00:19 <ildikov> #startmeeting cinder-nova-api-changes
16:00:24 <openstack> Meeting started Thu Aug 24 16:00:19 2017 UTC and is due to finish in 60 minutes.  The chair is ildikov. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:00:25 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:00:27 <openstack> The meeting name has been set to 'cinder_nova_api_changes'
16:00:32 <ildikov> johnthetubaguy jaypipes e0ne jgriffith hemna mriedem patrickeast smcginnis diablo_rojo xyang1 raj_singh lyarwood jungleboyj stvnoyes
16:00:41 <hemna> hi
16:00:44 <jungleboyj> @!
16:00:44 <mriedem> o/
16:00:44 <_pewp_> jungleboyj ヽ(´･ω･`)､
16:00:53 <smcginnis> o/
16:00:54 <jungleboyj> Though also in another meeting too.  :-)
16:01:22 <ildikov> jungleboyj: thanks for keeping an eye on this one too :)
16:01:32 <jungleboyj> ildikov:  I do what I can.  :-)
16:01:39 <ildikov> jungleboyj: :)
16:01:39 <stvnoyes> o/
16:02:14 <ildikov> ok, let's start
16:02:30 <ildikov> current open reviews: https://review.openstack.org/#/q/topic:bp/cinder-new-attach-apis
16:02:56 <smcginnis> https://review.openstack.org/#/q/topic:bp/cinder-new-attach-apis+status:open
16:02:59 <smcginnis> ;)
16:03:19 <ildikov> smcginnis: my bad :)
16:03:37 <smcginnis> Just a more concise view. :)
16:03:45 <ildikov> mriedem: the two on the top of the list are small changes to add an attachment_complete call and to tweak yet once more on the attachment_ref, mainly the connection_info part
16:04:01 <ildikov> mriedem: those can be reviewed already
16:04:32 <ildikov> for the attach patch we still need a client release to get a clean test run on the gate
16:04:40 <ildikov> locally things seem to work
16:05:05 <ildikov> except one thing that stvnoyes has just found with 'cinder show volume'
16:05:43 <smcginnis> ildikov: Latest on a new client release - looks like that will need to be next week.
16:05:53 <ildikov> with this patch we seem to use volume_id as opposed to attachment_id: https://github.com/openstack/python-cinderclient/commit/63ac82a55489d55246da939b5ae60b8a65fb9ec7
16:06:36 <ildikov> smcginnis: is that still issues with stable?
16:06:56 <ildikov> smcginnis: we might need to fix one more thing in the client, so from that perspective it sounds ok
16:07:04 <ildikov> smcginnis: I just wonder what else we have on the table
16:07:06 <smcginnis> ildikov: Yeah, the lib freeze will need to go until everything is complete for pike.
16:07:49 <ildikov> smcginnis: I guess we should be confident with end of next week then?
16:08:26 <smcginnis> ildikov: Better be, otherwise something went really wrong. ;)
16:08:48 <mriedem> i'd like more details in the commit message for https://review.openstack.org/#/c/493324/
16:08:52 <ildikov> smcginnis: my thinking as well, just wanted to be sure :)
16:09:03 <mriedem> ildikov: also, can you re-propose the spec for queens and make any necessary updates? https://review.openstack.org/#/c/373203/
16:09:12 <ildikov> mriedem: sure, will do
16:09:24 <mriedem> i guess i never got https://review.openstack.org/#/c/459134/ in
16:09:33 <ildikov> mriedem: yeah, I planned to, wanted to double check with you
16:10:10 <ildikov> mriedem: I will add that and I should add attachment_complete to it too
16:10:49 <mriedem> i've lost context on what my amendment is even about anymore
16:11:16 <mriedem> it was basically trying to capture discussion for something we'd need to consider when supporting multi-attach i think
16:12:17 <ildikov> it was about keeping begin_detaching with the new flow too, so we have the call in the API to put the volume into 'detaching' state as I remember
16:12:45 <ildikov> which is pretty much needed for multi-attach as we don't want two detaches going at the same time
16:13:45 <jgriffith> ildikov why not?
16:14:23 <jgriffith> We've again kinda lost sight of the whole point of discreet and independent attachment objects I think
16:14:42 <jgriffith> heck, at this point I'm not even sure I understand it any more :)
16:15:13 <ildikov> jgriffith: well, if we don't change the volume state that can be attach and etach as well that goes in parallel and I'm not sure that's a good idea
16:15:38 <ildikov> jgriffith: and the volume state machine doesn't support it either
16:15:46 <jgriffith> probably not a discussion for right now, but that whole thing needs to go away/stop
16:15:50 <ildikov> so for now it's better not to create a mess :)
16:15:56 <jgriffith> the attachment needs to be the source of truth
16:16:17 <ildikov> it sounds good for the long run
16:16:30 <ildikov> but we can't start those changes in Nova
16:16:38 <jgriffith> understood
16:16:54 <jgriffith> but don't try and cheat and force in a half assed multi-attach solution either :)
16:17:10 <mriedem> ildikov: how about we just re-propose the spec and leave out https://review.openstack.org/#/c/459134/
16:17:16 <mriedem> we can roll that into the queens spec later if we want
16:17:21 <jgriffith> anyway, not sure it's really relevant quite yet... sorry
16:17:28 <jgriffith> don't want to derail
16:17:30 <mriedem> right let's avoid the noise
16:17:34 <mriedem> and just ignore https://review.openstack.org/#/c/459134/1
16:17:38 <ildikov> jgriffith: we will talk about the multi-attach bits on the PTG, I hope :)
16:17:55 <ildikov> mriedem: works for me
16:18:04 <ildikov> mriedem: thanks for the hint
16:18:17 <ildikov> jgriffith: BTW, did you see my above comment on cinder show volume not returning the correct attachment_id?
16:18:31 <jgriffith> No
16:18:44 <ildikov> jgriffith: stvnoyes has just found out as he uses that for one of his tempest tests
16:18:48 <smcginnis> mriedem: Do we have a good time for a joint PTG session?
16:18:55 <smcginnis> jungleboyj: ^
16:18:56 <mriedem> for a good time...
16:19:00 <mriedem> like on a bathroom wall?
16:19:16 <mriedem> smcginnis: anytime wed-fri is fine
16:19:32 <mriedem> we just have an etherpad with proposed stuff right now, nothing in order
16:19:40 <ildikov> jgriffith: so this is apparently the volume_id for some reason: https://github.com/openstack/python-cinderclient/commit/63ac82a55489d55246da939b5ae60b8a65fb9ec7#diff-799a44cd44f14a12e4f1d2a8b56ab63aR39
16:19:48 <smcginnis> mriedem: Same for us.
16:19:57 <jungleboyj> :-)  mriedem  We should coordinate that.
16:20:07 <mriedem> i hope we're not in the same wedding ballrooms as last time
16:20:09 <ildikov> smcginnis: thanks for bringing it up, so I don't forget :)
16:20:28 <smcginnis> ildikov: I've been meaning to for a while, so I figured I better while we're all here. :)
16:20:29 <jungleboyj> mriedem:  Do you have a preference.  Maybe Thursday some time?
16:20:34 <mriedem> jungleboyj: that works
16:20:37 <jgriffith> wedding?  who's getting hitched?  Free food and drink.. YAY
16:20:37 <mriedem> thursday morning
16:20:40 <smcginnis> ++
16:20:45 <jungleboyj> jgriffith:  ++
16:20:50 <smcginnis> Was just going to suggest morning so there's follow on time.
16:20:50 <mriedem> i just want a room with a ceiling that's <20 feet
16:20:57 <jungleboyj> mriedem:  That sounds good.
16:21:03 <ildikov> sounds good
16:21:26 <jungleboyj> jgriffith: ildikov  You guys will get everyone up to speed on where the API rework and impacts during that time?
16:21:58 <jungleboyj> mriedem:  Is there interest on the Nova side there?  I know that the Cinder team needs to be updated.
16:22:16 * smcginnis has to drop off for a bit for solo parent duties
16:22:32 <ildikov> jungleboyj: that's an option, but it would be more high level if we have both teams there
16:22:54 <mriedem> i'd be happy if more nova people were involved
16:23:05 <ildikov> mriedem: +2 +A
16:23:09 <mriedem> if johnthetubaguy is going to be at the ptg he'd need to be updated
16:23:15 <mriedem> otherwise maybe gibi will get the mantle
16:23:24 <jungleboyj> johnthetubaguy:  You going to be there
16:23:35 * jungleboyj whispers "please say yes, please say yes"
16:23:37 <ildikov> I think he has a ton of concerns without update too :)
16:23:50 <mriedem> yeah anyway
16:23:52 <diablo_rojo_phon> He is going to be there.
16:24:00 <jungleboyj> Ok, that is good news.
16:24:06 <mriedem> what next
16:24:12 * mriedem still has rc2 bugs to fix
16:24:33 <ildikov> mriedem: yeah, I gave gibi an idea on how annoying I can be with review requests in the second line after 'congrats'... :)
16:24:52 <ildikov> mriedem: not much more on the Nova side until we don't have the cinderclient fixed and released
16:25:05 <mriedem> ok
16:25:26 <ildikov> jgriffith: so as for volume show, it seems to be an easy client side fix
16:25:30 <mriedem> maybe i'll throw https://review.openstack.org/#/c/463987/ in next week
16:25:44 <mriedem> stvnoyes: is ^ still happy with the new flow?
16:25:58 <mriedem> or i guess we can't test that in upstream CI until cinderclient fixes are released
16:26:05 <stvnoyes> working thru that cinder client issue atm
16:26:18 <mriedem> ok, because the stack of api changes at https://review.openstack.org/#/c/330285/ depends on the cinderclient fixes
16:26:24 <mriedem> so i guess i'll hold off
16:27:03 <ildikov> jgriffith: it's weird why we have the volume_id as the value of 'id' under 'attachments' in the volume though, at least for my taste
16:27:15 <ildikov> jgriffith: maybe hemna has the history of that
16:27:36 <ildikov> mriedem: well, things mostly look good locally, but I guess it sounds less convincing anyway
16:28:33 <jgriffith> ildikov there's a lot of garbage in there frankly
16:28:59 <jgriffith> like storing attach-status in 3 different places etc; I'll look at the volume-show stuff you mentioned and see what's up
16:29:00 <mriedem> the attachment_id in the compute API for attached volumes has always been the volume_id i think
16:29:35 <stvnoyes> jgriffith-  in cinderclient.v2.shell.py._translate_attachments, the attachment_id is replaced with attachment['id'] which happens to be the volume id
16:29:45 <jgriffith> mriedem I believe you're correct on that
16:29:49 <mriedem> https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/volumes.py#L226
16:29:53 <ildikov> jgriffith: I think for now we can just use the attachment_id in the translate function in the client
16:29:55 <mriedem> super legacy
16:29:58 <stvnoyes> the correct attachment id is in 'attachment_id'
16:30:11 <stvnoyes> not id
16:30:19 <jgriffith> stvnoyes got ya
16:30:22 <mriedem> right, my point is it's probably there fore super old legacy reason
16:30:24 <mriedem> like https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/volumes.py#L226
16:30:32 <mriedem> when cinder wasn't a thing, it was nova-volume
16:30:47 <jgriffith> mriedem +1 you're correct
16:30:50 <jgriffith> n-vol thing
16:30:58 <ildikov> mriedem: ok, then we need to fix that too
16:31:22 <stvnoyes> so this show issue may have been introduced with that translate_attachments change...
16:31:43 <stvnoyes> it was working ok earlier.
16:32:32 <jgriffith> stvnoyes yeah, I made some changes to cinderclient that might be culprit too though.  But yes it worked beautifully for a while :)
16:33:04 <jgriffith> anyway... i think this is a Cinder show thing that needs to be fixed and I don't want to pile on things regarding changes to legacy stuff until we finish what's in flight
16:33:28 <jgriffith> there are too many things up in the air right now IMO
16:33:57 <ildikov> jgriffith: we still need the correct data in the volume object too
16:34:36 <ildikov> jgriffith: so we can double check the client, we have a bit more time till the lib freeze is finally over
16:35:56 <ildikov> anything else to the topic?
16:36:54 <mriedem> nope
16:37:11 <ildikov> ok, then I think that was it for today
16:37:38 <ildikov> unless someone has further questions/comments to discuss
16:38:26 <ildikov> ok, we can figure out topics for the joint session on the PTG offline and keep fixing up the cinderclient
16:38:31 <ildikov> thanks everyone!
16:38:41 <ildikov> C U here next week
16:38:43 <hemna> ildikov, that volume_id thing I believe was a placeholder that has been there from the n-vol days, I just left it there to not break anything.
16:39:51 <ildikov> hemna: sounds good, we have the attachment_id explicitly there, so we can fall back to that and do cleanups later
16:40:15 <ildikov> would be my thinking re not breaking anything
16:41:36 <ildikov> alright, have a good rest of the day everyone :)
16:42:09 <ildikov> #endmeeting