17:00:09 <ildikov> #startmeeting cinder-nova-api-changes
17:00:10 <openstack> Meeting started Mon Sep 19 17:00:09 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:12 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:00:15 <openstack> The meeting name has been set to 'cinder_nova_api_changes'
17:00:18 <ildikov> scottda ildikov DuncanT ameade cFouts johnthetubaguy jaypipes takashin alaski e0ne jgriffith tbarron andrearosa hemna erlon mriedem gouthamr ebalduf patrickeast smcginnis diablo_rojo gsilvis
17:01:59 <scottda> hi
17:03:01 <mriedem> o/
17:04:12 <ildikov> hi all, finally no RC1 or holidays so we can have the series and activities going :)
17:04:23 <ildikov> as a reminder here's our etherpad: https://etherpad.openstack.org/p/cinder-nova-api-changes
17:04:48 <ildikov> I added the links to open reviews so that we can have a better picture on what we started already
17:04:59 * johnthetubaguy is lurking, just finishing another call
17:05:06 <ildikov> jgriffith: do you have any updates regarding the spec you planned to write up?
17:07:12 <johnthetubaguy> +1 on that question
17:07:51 <ildikov> while we have jgriffith to arrive, I would also like to discuss what we need to be prepared for the Design Summit and the upcoming short cycle
17:08:24 <ildikov> it would be great to identify a high level roadmap and discuss more specifics in person I think
17:08:45 <scottda> In the past we've had a cinder-nova cross project session. I'm not sure if we need one for Barcelona?
17:09:12 <mriedem> i assumed there would be a nova/cinder design summit session, but i wasn't sure
17:09:13 <ildikov> I also saw on the Nova retrospective etherpad that it's not clear for everyone what we are working on, so I would also like to know what we could do to clarify it to people?
17:09:27 <johnthetubaguy> so I would love to get cinder and neutron on the same page about live-migrate and APIs
17:09:38 <scottda> Regarding the O cycle, Cinder is leaning towards a "stability" cycle, with emphasis on bug-fixing and testing.
17:09:47 <scottda> johnthetubaguy: +1 to that.
17:12:06 <mriedem> nova isn't
17:12:12 <scottda> johnthetubaguy: I think you have Nova specs up for API changes, don't you? Could you put links in the cinder-nova-api-changes etherpad?
17:12:30 <johnthetubaguy> not good ones
17:12:34 <johnthetubaguy> I need to refine them (this week)
17:13:15 <ildikov> scottda: does this mean that the api work jgriffith started is not in scope for Ocata in Cinder?
17:13:38 <scottda> johnthetubaguy: OK, well even the starting point would be nice to link it. At some point, we'll need to start pointing more people to what we are doing to give context for discussions.
17:13:39 <ildikov> mriedem: +1 one on the session, I hoped to have one
17:14:14 <scottda> ildikov: I'm not sure. And this was just a campaign promise by smcginnis, who probaly has zero chance of getting re-elected PTL anyway :)
17:14:23 <hemna> hey
17:14:26 <hemna> sorry was in another meeting
17:15:06 <ildikov> scottda: haha, thanks for the insights :)
17:15:25 <hemna> I still think we need to have a cross project working group for nova-cinder until we iron out the new cinder API and it's ramifications in nova
17:15:41 <scottda> ildikov: I'd bet we can move this forward in Ocata. But we might not get as much done as we'd like.
17:16:01 <ildikov> so I hope we can move on to have that experimental API in Cinder, especially that there's already code up for it and mriedem started to get the gate working as well
17:16:08 <hemna> I'd like to see jgriffith's patch land early in O and get a nova patch in gerrit to test against
17:16:13 <johnthetubaguy> scottda: yeah, let me find that, one sec, out of other meeting now
17:16:16 <ildikov> hemna: +1
17:16:26 <hemna> ildikov, one of the things we spoke about in the cinder mid cycle was doing a feature branch
17:16:33 <hemna> in cinder for this instead of an experimental api
17:16:58 <johnthetubaguy> so this is my mega spec on neutorn changes, it will die very soon and split into many little specs: https://review.openstack.org/#/c/353982/
17:17:00 <hemna> as experimental api's still make it into the wild
17:17:21 <johnthetubaguy> this is the existing spec around the neutron api changes: https://review.openstack.org/#/c/309416
17:17:54 <mriedem> i've never worked on a feature branch so i'm not sure how much trouble that is wrt the infra for running ci and chaining changes together
17:18:24 <johnthetubaguy> my worry is about getting from the feature branch back into master
17:18:36 <scottda> hemna: I don't recall the outcome of presenting options (experimental API vs. feature vs. ?) to Nova, but I do remember there were strong opinions about one option...
17:18:39 <hemna> merge conflicts, etc?
17:19:26 <hemna> so yah, this is something we should talk about in BCN and see what makes the most sense
17:19:55 <ildikov> hemna: I thought we dropped the feature branch idea there as a conclusion
17:19:55 <hemna> we need some bake time in with the cinder api changes and see how it works with nova
17:20:10 <scottda> Line#103 https://etherpad.openstack.org/p/newton-cinder-midcycle-day2 for notes on that..
17:20:28 <hemna> ildikov, we might have, I just forgot what we decided
17:21:03 <scottda> Looks like nothing marked as Resolved on the issue of experimental APi vs. Feature at the mid-cycle.
17:21:05 <ildikov> I would rather go to the experimental direction, as it is a new API it should not do that much harm, but it is just my view
17:21:26 <hemna> ildikov, so the one argument against that is that the API still gets released to the wild
17:21:39 <hemna> and folks start to use it....
17:21:41 <johnthetubaguy> so we have being doing the placement API in tree in Nova, not get enough data on that yet I guess
17:21:44 <hemna> and then complain that it goes away
17:21:52 <johnthetubaguy> although thats a bit of a different case I guess
17:22:08 <scottda> ildikov: I'm ok with experimental direction. And I could do the code. We'd want to figure this out before the summit, to discuss with Cinder (we've discussed this one in Tokyo, mid-cyle, ...)
17:22:44 <hemna> honestly, this is why we have branches in git
17:23:14 <ildikov> hemna: as jgriffith already has code, can we figure out until the Summit how likely that will go away?
17:23:29 <hemna> go away?
17:23:51 <ildikov> hemna: I think the Cinder API design should be on one hand simple and on the other hand good for Cinder's use cases
17:24:03 <hemna> so, I had planned on putting a patch together in the python-brick-cinderclient-ext to use the new API that john has
17:24:13 <ildikov> hemna: you had the argument against the experimental that people will start to use it and then it'll go away
17:24:21 <hemna> this will allow us to test against direct attaching inside a vm or a bare metal node
17:24:26 <hemna> without nova
17:25:12 <scottda> hemna: Cool, that will be good. We still need to figure how to POC and test with Nova.
17:25:22 <hemna> ildikov, so I don't think his patch will go away, but I think the api may/will change as we start to implement against it in nova and cinderclient
17:25:55 <hemna> either way, we need to get that code to land somewhere and start using it
17:26:29 <mriedem> to be fair john has nova patches using it today
17:26:31 <mriedem> via cinderclient
17:26:49 <ildikov> hemna: I think the nest thing as they way forward to keep things simple and remove workarounds in Nova, so I hope a basic attach/detach API in Cinder does not get changed that much because of the interactions with Nova
17:27:58 <hemna> it's hard to tell until we start using it
17:28:15 <scottda> Yes, perhaps we're just being paranoid about the flexibility to change the API...
17:28:18 <hemna> and introducing live migration, shelve, etc into the mix
17:28:55 <hemna> and we should think about how it fits into multi-attach as well
17:28:56 <scottda> I say this as someone who is in favor of experimental APIs. But there are many who oppose it, so might be tough to get consensus.
17:28:59 <hemna> and what we want to do there
17:29:22 <ildikov> hemna: if we end up overcomplicating the Cinder API because of those use cases then we might do something wrong, but that's just my 2 cents, I can move on :)
17:29:40 <hemna> well, that's kinda how we got here.
17:29:50 <hemna> is that we have a complicated API to account for those cases
17:30:12 <ildikov> we have a let's use the ugly word legacy API, that we're I hope ready to change now
17:30:20 <scottda> ildikov: +1. That is how we got here. I'd rather see a separate API for corner cases like shelve offload, etc. That will hopefully prevent the coding errors we seen.
17:31:04 <ildikov> if we expect that things will not need to be changed and cleaned up Nova for instance, then it's not the best direction we can go to IMHO, I would like to avoid that
17:31:17 <ildikov> *in Nova
17:31:23 <johnthetubaguy> the way I like to think about this, is we need to stop telling each other lies, and be more explicit about things
17:31:39 <scottda> johnthetubaguy: Agreed.
17:31:48 <mriedem> unless that was a lie
17:32:10 * johnthetubaguy head goes boom
17:32:14 <scottda> But, my opinion is that one reason for the lies is that Cinder keeps attach state, which is of use to Nova, but not necessarily important for Cinder.
17:32:16 <hemna> liars!
17:32:27 <ildikov> johnthetubaguy: +1
17:32:35 <scottda> And Nova is the only entity that really knows if a volume is attached.
17:32:36 <hemna> scottda, well but it is important for cinder too
17:32:48 <ildikov> scottda: we are removing that from Nova
17:32:53 <hemna> ildikov, +1
17:33:09 <hemna> nova shouldn't care, it should just ask cinder to do something
17:33:15 <hemna> and cinder can say, ok, or no.
17:33:33 <scottda> Yeah, I wont' go down this path again. I think Cinder shouldn't care, it should just provide an export and say "do with it as you please"
17:33:36 <ildikov> scottda: the whole point of removing check_attach from Nova is to stop it from following a state that it should not use for any internal decisions really
17:33:41 <hemna> it's cinder's job to keep track of it's volumes and what's allowed or not
17:33:42 <scottda> And be nice, and tell us when you are done
17:34:25 <scottda> But then we're vulnerable to lies
17:34:34 <hemna> the problem is Cinder can be doing things to the volume that should prevent nova from attaching it at the time.....think migration or retyping, etc.
17:34:39 <scottda> (if cinder keeps track of volumes, and what's allowed or not)
17:35:05 <johnthetubaguy> hemna: right I think thats the problem, Nova needs explicit (and exclusive) permission for what it wants to do
17:35:23 <scottda> yeah, OK. We're painted in the corner on those cases.
17:35:47 <hemna> johnthetubaguy, yah it does and it will with the new api, just behind the scenes in Cinder
17:36:02 <johnthetubaguy> hemna: +1
17:36:04 <hemna> nova just asks, and it gets a yes or no answer
17:36:09 <scottda> Anyway, we can solve the problem of having to lie for special cases by having a separate API for the unusual Nova cases.
17:36:31 <scottda> i.e detach, shelve_offload_detach, live_migration_detach, or something like that.
17:36:40 <scottda> Keep things explicit, to avoid coding errors.
17:36:50 <hemna> https://review.openstack.org/#/c/327408/
17:36:56 <hemna> just FYI, the new API patch
17:36:57 <ildikov> hemna: it's what reserve_volume does currently as well, right?
17:37:05 <hemna> ildikov, yah
17:37:11 <ildikov> hemna: I mean letting Nova know it can use the volume or not
17:37:24 <hemna> yup
17:37:33 <hemna> it's far simpler and much less racy as well
17:37:48 <ildikov> scottda: I think jgriffith also mentioned a safe_to_detach call on the API or smth like as one way to handle corner cases
17:38:13 <hemna> dulko was even asking about making that patch experimental
17:38:28 <scottda> ildikov: Yes, that sound about right, but there is still a need to do things for migration like "detach the volume, but don't change the state in the Cinder DB"
17:38:29 <ildikov> hemna: scottda: yup, I think wherever we do this tracking that should be at one place and not in both
17:38:48 <ildikov> hemna: scottda: I lean towards what hemna says and what reserve_volume does today
17:39:07 <ildikov> we can have a separate meeting/chat about this topic if it's still an open question in Cinder
17:39:17 <ildikov> I think it's very important to address and agree on
17:39:25 <scottda> Well, it will come up when we move past just attach/detach
17:39:56 <ildikov> scottda: right, we need to think about additional cases as well, I agree, I only wanted to mention an example for now
17:40:29 <johnthetubaguy> I would like live-migration to be two attachments, to the same instance, but on a different host
17:40:43 <johnthetubaguy> largely as that seems to be the way neutron is going with port bindings
17:40:51 <hemna> johnthetubaguy, yup that's exactly what it is
17:41:09 <johnthetubaguy> hemna: good stuff
17:41:11 <hemna> nova currently just bypasses the official end to end attach process for the 2nd attachment.
17:41:19 <scottda> johnthetubaguy: That's fine. But we will not be able to use new_attach api, we'll need a separate one.
17:41:46 <hemna> is there any way to change the order at all?
17:41:53 <hemna> so there is only 1 attach at a time ?
17:41:59 <hemna> I know, that's crazy talk
17:42:02 <johnthetubaguy> depends
17:42:05 <johnthetubaguy> ah, so no
17:42:07 <scottda> live_migration_attach. Which does the same as now regarding attaching to both, but doesn't fail on Cinder DB state.
17:42:26 <hemna> I guess the problem is we can't tear down the source before we bring the destination n-cpu instance up
17:42:45 <johnthetubaguy> basically, live-migrate involves one VM being fully connected to all ports and volumes, on two different hosts
17:42:50 <ildikov> I don't know whether we should go down the rabbit hole of using flags
17:42:52 <scottda> And tricky for Boot from Volume, I believe
17:42:54 <ildikov> most probably not
17:42:55 <johnthetubaguy> hemna: yeah, spot in
17:43:16 <ildikov> johnthetubaguy: is this planned out for Neutron already?
17:43:26 <ildikov> johnthetubaguy: I mean the migration use case for ports
17:43:33 <scottda> ildikov: -1 to flags. I think people get confused and write buggy code when we re-use the API for too much.
17:43:46 <hemna> scottda, +1
17:43:54 <johnthetubaguy> ildikov: possibly, may only get the neutorn side done, but hopefully make progress on both
17:44:26 <ildikov> scottda: +1, I had the same thought, just wanted to be sure we all agree
17:44:27 <johnthetubaguy> so an attachement is always for a specific host and specific device(instance uuid)
17:44:48 <johnthetubaguy> it seem we can just have two of those now, but how we want to show that in the API is a different thing
17:44:58 <ildikov> johnthetubaguy: I was wondering more about the direction you picked there to try to use the same/similar concept if possible
17:45:08 <johnthetubaguy> multi-attach is two instance uuids, and live-migrate is two host uuids
17:45:40 <hemna> the new API doesn't have host
17:45:43 <hemna> it's just instance_uuid
17:45:43 <johnthetubaguy> ildikov: so neutron is looking at port binding, were we can have two of them at once, and we tell neutron which is the active one
17:45:52 <hemna> the host is inside the connector
17:46:06 <johnthetubaguy> I thought host was going to be required to get sent from Nova
17:46:18 <scottda> hemna: That's why we need live_migration_second_attach(instance_id, host1, host2,..) or something like that.
17:46:20 <johnthetubaguy> so we can do connection tracking for the multi-attache cases
17:46:22 <hemna> create_attachment(self, context, volume_ref, connector, instance_uuid, mountpoint, no_connect=False):
17:46:34 <ildikov> johnthetubaguy: ok, I see
17:46:41 <hemna> so technically the host is in there, just inside the connector dict
17:46:48 <hemna> it's just not explicit
17:46:49 <scottda> plus live_migration_detach_old_host(blah, blah)
17:46:57 <johnthetubaguy> that seems odd, its a property of the attachement
17:47:01 <hemna> yah
17:47:07 <hemna> I'd like to see it called out separately as well
17:47:15 <hemna> which also makes sense for bare metal attaches
17:47:27 <johnthetubaguy> I mean, how its stored in the DB doesn't matter to me, I am just thinking conceptually
17:47:29 <scottda> Well, we're changing the API, so call it out separately if we want
17:47:30 <hemna> nova can just pull it out of the connector prior to calling the api
17:47:33 <ildikov> johnthetubaguy: I thought Cinder will keep the connector info from now on and use the data in it
17:47:39 <ildikov> or smth similar :)
17:47:59 <hemna> ildikov, it should be saved in the attachment table.
17:48:05 <hemna> when the new api is called it should save the connector
17:48:18 <hemna> which also solves a lot of our shelve and force detach issues
17:48:34 <johnthetubaguy> right, for each (volume_uuid, host_uuid) you need to store a connector info, I would have thought?
17:48:41 <hemna> johnthetubaguy, +1
17:48:48 <hemna> yah that should be part of this
17:48:53 <scottda> hemna: I believe jgriffith 's patches do save the connector...
17:49:00 <hemna> I haven't spent a lot of time reviewing the patch
17:49:08 <hemna> but it should save the connector.
17:49:33 <ildikov> hemna: right, this is what I had in mind
17:49:53 <johnthetubaguy> I think the connector is saved
17:50:09 <johnthetubaguy> I just worry live-migrate will wonder in an break things, unless its per volume_uuid and host_uuid
17:50:31 <johnthetubaguy> I am thinking about a multi-attached volume that is doing a live-migrate, so it gets three attachements
17:50:33 <ildikov> hemna: as Cinder currently does not store the info and Nova has the reoccurring initialize_connection calls that we want to remove as well IIRC
17:51:04 <hemna> ok so, I'll go through the patch and see if I can track down the connector
17:51:08 <hemna> I'm not sure it's saving it actually
17:51:15 <hemna> https://review.openstack.org/#/c/327408/13/cinder/volume/manager.py
17:51:22 <hemna> I don't see it getting saved
17:51:30 <scottda> hemna: Yeah, I'm scanning now and don't see it save either
17:51:31 <hemna> anyway, that can be changed
17:52:12 <hemna> so, I think we'll have to add a new DB column in the volume_attachment table
17:52:16 <hemna> to store the connector.
17:52:17 <johnthetubaguy> sounds like we are converging on the concepts at least
17:52:28 <ildikov> johnthetubaguy: +1 :)
17:52:50 <hemna> http://paste.openstack.org/show/581172/
17:53:02 <hemna> that's all that the attachment entry has right now
17:53:05 <johnthetubaguy> seems like we need the writing up ahead of the summit, so we can agree the details at the summit?
17:53:16 <hemna> johnthetubaguy, not a bad idea
17:53:38 <ildikov> johnthetubaguy: exactly my thought as well
17:54:20 <ildikov> I will add the main topics we touched today to the top of our etherpad and we can go through them before the Summit to prepare, if that sounds good
17:55:01 <ildikov> I would like to keep this meeting series too and use as preparation because it seems we still have a lot to agree on
17:55:09 <hemna> yup
17:55:31 <hemna> I think there will be lots of iron out once we get the cinder api in place.  I think the changes in Nova are going to be complex.
17:55:36 <scottda> +1 keep meeting
17:55:41 <ildikov> does it sound ok to everyone as next step(s)?
17:55:46 <hemna> as Nova will have to maintain compatibility with the old API for a while.
17:55:52 <ildikov> hemna: +1
17:55:52 <hemna> ildikov, +!
17:55:56 <hemna> +1 even
17:56:38 <ildikov> although I hope we can simplify things in Nova too, or at least "clarify" and have less hacky things
17:56:40 <scottda> +1
17:56:48 <hemna> johnthetubaguy, one of the changes involved is that the new Cinder API will take a while to complete
17:57:06 <hemna> and Nova currently calls reserve_volume way early in the process (nova api) before it continues.
17:57:22 <hemna> so, if the nova api changes to call the new cinder api, that might take a while
17:57:37 <johnthetubaguy> ah, so reserve_volume is changing quite a lot?
17:57:49 <hemna> as cinder will go all the way to the backend (c-vol -> driver) and back before it responds.
17:57:59 <johnthetubaguy> ah...
17:58:00 <scottda> johnthetubaguy: reserve_volume goes away...
17:58:15 <scottda> johnthetubaguy: And all the attach calls merge into 1 call
17:58:15 <johnthetubaguy> hmm, I had missed that being suggested
17:58:21 <hemna> well, the new API internalls calls reserve_volume, but then also does an RPC to the c-vol service to call initialize_Connection, which calls the cinder backend.
17:58:33 <ildikov> it does? I wasn't sure we decided that reserve_volume goes away too
17:58:42 <hemna> well reserve_volume exists
17:58:42 <mriedem> i wasn't sure that reserve goes away either
17:58:47 <hemna> but nova isn't supposed to call it
17:58:50 <johnthetubaguy> so just as a heads up, that does make Nova's API experience a bit worse
17:58:53 <hemna> all it does is call create_Attachment
17:58:57 <mriedem> i thought nova-api was going to call reserve as it does today
17:58:59 <scottda> nova doesn't call it.
17:59:00 <johnthetubaguy> but honestly, I am OK with that
17:59:10 <hemna> no, it just calls create_attachment
17:59:11 <mriedem> and i thought initialize/attach were 1 api that n-cpu would call
17:59:19 <hemna> https://review.openstack.org/#/c/327408/
17:59:25 <hemna> read the commit message
17:59:29 <johnthetubaguy> so in the API we want to error early if there are issues, but most of the time there will be no issues, so maybe its OK
17:59:33 <ildikov> hmm, I thought we keep reserve_volume there to ensure Nova gets the green/red light early on and then let the heavier flow complete later
17:59:35 <hemna> it replaces 1. 2. 3 with create_attachment()
17:59:47 <mriedem> hemna: https://review.openstack.org/#/c/330285/6/nova/compute/api.py
17:59:49 <johnthetubaguy> ildikov: right, thats where I was with that
17:59:52 <mriedem> that's not what the nova patch does today
17:59:55 <hemna> johnthetubaguy, so if the volume is in a state where it can't attach, it will still error early
17:59:56 <mriedem> it still calls reserve
18:00:00 <ildikov> hemna: I think jgriffith said at a certain point that reserve_volume might stay, but he didn't update the commit message
18:00:17 <jgriffith> sorry... just got back to desk
18:00:17 <johnthetubaguy> hemna: its about the API returning really quickly always
18:00:22 <johnthetubaguy> so we can call it in the API
18:00:24 <hemna> the issue comes up when the volume can be attached, then c-api RPC's to c-vol to initialize_connection and then calls the cinder backend
18:00:27 <jgriffith> so quick backscroll... :)
18:00:33 <jgriffith> Yes, I save the connector info
18:00:35 <ildikov> jgriffith: right in time, we ran out of the meeting time :)
18:00:39 <hemna> I just wanted to raise that issue
18:00:43 <hemna> as it's a change
18:00:43 <jgriffith> Yes, plan to keep reserve
18:00:55 <jgriffith> those thing are in the patch sets that are up
18:00:59 <johnthetubaguy> cool, keeping that as a quick operation would really help us
18:01:06 <johnthetubaguy> jgriffith: did you get that spec written up :)
18:01:07 <ildikov> let's switch to the Cinder channel
18:01:18 <jgriffith> Also, now that Ocata is open for biz I'm working on updates to finish those patches
18:01:26 <ildikov> and see you next week for the meeting here!
18:01:32 <scottda> Is there another meeting here
18:01:32 <scottda> ?
18:01:33 <hemna> jgriffith, ok cool.
18:01:35 <jgriffith> sorry.. ok, cya in Cinder
18:01:40 <scottda> We might not get booted out of this room...
18:01:51 <johnthetubaguy> right, I was thinking someone would have told us off already
18:01:56 <ildikov> scottda: I'm not sure, i don't have the calendar/meeting page open
18:02:24 <johnthetubaguy> so I have to go cook my dinner really soon
18:02:33 <scottda> ok, john already moved.
18:02:40 <scottda> Don't forget to #endmeeting
18:02:42 <johnthetubaguy> mostly because I am getting hungry...
18:03:27 <scottda> ildikov: ^^^
18:03:35 <ildikov> johnthetubaguy: I have a plane to catch... :)
18:03:49 <johnthetubaguy> ildikov: that totally wins
18:03:54 <johnthetubaguy> (just)
18:04:05 <ildikov> scottda: will do it now as the guys are chatting on the Cinder channel now, thanks for reminding! :)
18:04:08 <ildikov> johnthetubaguy: :)
18:04:12 <ildikov> #endmeeting