16:00:13 <ildikov> #startmeeting cinder-nova-api-changes
16:00:17 <openstack> Meeting started Thu Apr 13 16:00:13 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:19 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:00:21 <openstack> The meeting name has been set to 'cinder_nova_api_changes'
16:00:39 <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 breitz jungleboyj
16:00:49 <jungleboyj> o/
16:00:50 <smcginnis> ./
16:00:53 <tommylikehu> o/
16:01:02 <hemna> o/
16:01:08 <stvnoyes> o/
16:02:08 <ildikov> hi all
16:02:22 <ildikov> let me check whether we have anyone for today from Nova
16:02:37 * johnthetubaguy lurks
16:02:45 <johnthetubaguy> so its spec freeze day today
16:03:03 <mriedem> o/
16:03:08 <ildikov> johnthetubaguy: ah, I see, I knew there was something :)
16:03:12 <lyarwood> o/
16:03:49 <ildikov> I have only a few items for today
16:04:17 <ildikov> johnthetubaguy: I uploaded a new version of check detach with an updated commit message
16:05:02 <ildikov> I believe that's where we got last week with that discussion
16:05:13 <ildikov> I tried to add more info about the change
16:05:57 <ildikov> I also started to look into the detach patch
16:06:22 <ildikov> as it's an older one we need to look into cases like swap and post migrate
16:06:54 <ildikov> I thought to move those changes to a follow up patch and have those changes included in the first one that are ready to merge
16:07:11 <ildikov> mriedem: johnthetubaguy: lyarwood: would that make sense? ^
16:07:16 <mriedem> i think we said we'd handle the different operations in different patches
16:07:27 <mriedem> since none of it turns on until we do attach with new flow at the end
16:07:40 <mriedem> so let's just handle normal volume detach in that change
16:08:15 <ildikov> ok, sounds good, I will separate those then
16:08:24 <lyarwood> agreed, this should also help spread the load here a little, I wasn't sure who was working on this detach change tbh
16:09:16 <mriedem> i would definitely move this pattern into a private method in the compute manager
16:09:18 <mriedem> if bdm.is_volume:
16:09:18 <ildikov> lyarwood: we try to work together with John (Griffith) except when we get overloaded at the same time...
16:09:18 <mriedem> if not bdm.attachment_id:
16:09:20 <mriedem> else:
16:09:28 <mriedem> self.volume_api.attachment_update
16:09:30 <mriedem> or whatever
16:09:34 <mriedem> that's duplicated everywhere
16:09:40 <mriedem> let's do that in a single private method
16:09:47 <ildikov> lyarwood: as these changes can be independent it's easy to spread the load
16:10:12 <lyarwood> ildikov: yup agreed
16:10:22 <ildikov> mriedem: that one BTW will be a delete - create combo, but got your point
16:11:12 <johnthetubaguy> mriedem: +1
16:11:28 <ildikov> lyarwood: would your planned change help with what mriedem is pointing out above? ^
16:11:56 <mriedem> i think just using a simple private method to contain that if/else/if logic will help
16:12:05 <johnthetubaguy> mriedem: I was wondering if we want a DNM patch at the end that does that new style attach, in an attempt to get test coverage sooner? Maybe that just makes things messier?
16:12:06 <mriedem> rather than overhauling and moving everything to the block_device module before this
16:12:35 <mriedem> johnthetubaguy: idk at this point
16:13:15 <ildikov> mriedem: ok, got it
16:13:25 <lyarwood> I can look into a DNM attach change next week if there are no other takers
16:13:36 <ildikov> johnthetubaguy: I can try to look into how much the earlier attach PoC works still
16:14:02 <ildikov> johnthetubaguy: we had it working at some point, but I believe that was before merging the new Cinder API
16:14:25 <mriedem> i guess we don't detach on shelve offload huh
16:14:32 <mriedem> i suppose that makes sense
16:14:42 <mriedem> we need the ports and volumes when we unshelve
16:14:58 <mriedem> although, when you unshelve, you're building on a new host (maybe)
16:14:58 <ildikov> johnthetubaguy: there's also some bug fixing activity driven by tommylikehu that has API changes, like rename instance_id to server_id
16:15:12 <mriedem> API changes?
16:15:12 <tommylikehu> ildikov:  yes
16:15:29 <ildikov> mriedem: if we have an empty attachment there that's just reserving the volume I guess that would do
16:15:33 <mriedem> so another microversion for key changes in the payload?
16:16:27 <tommylikehu> I am not sure whether we need another micrioversion if the attachment APis haven't been truely used
16:16:28 <ildikov> as we don't use any microversion at this point I would think to just use the latest that contains what we need?
16:16:37 <mriedem> tommylikehu: you do
16:16:50 <mriedem> ildikov: we use 3.27 or whatever
16:17:00 <ildikov> mriedem: not at this point
16:17:03 <mriedem> tommylikehu: it's unsafe to make assumptions about who is using an api once you release it
16:17:12 <mriedem> ildikov: i mean, that's the minimum we can use, from ocata
16:17:15 <tommylikehu> mriedem: ok
16:17:46 <johnthetubaguy> mriedem: +1, sounds like a new microversion to me
16:18:00 <mriedem> seems like a waste of a microversion for a cosmetic change like that
16:18:01 <ildikov> mriedem: BTW, is that configurable?
16:18:07 <mriedem> ildikov: is what configurable?
16:18:21 <ildikov> what microversion Nova is using
16:18:26 <mriedem> no
16:18:47 <ildikov> ok
16:18:55 <ildikov> that's what I thought
16:18:55 <mriedem> remember, configuration changes which impact how apis work are the devil
16:19:05 <johnthetubaguy> I think we should support just one micro-version for the new flow, its complicated if not
16:19:11 <mriedem> johnthetubaguy: yes
16:19:14 <mriedem> 3.27 or bust
16:19:17 <mriedem> until we need something newer
16:19:28 <ildikov> I know, I just got lost on the another microversion in the payload comment
16:19:30 <mriedem> anyway, we're not there yet
16:19:49 <mriedem> i need to remind myself at some point why we don't terminate connections during shelve offload
16:20:02 <mriedem> because that seems important when you unshelve and the instance is built on another host
16:20:10 <ildikov> so if we would end up do the cosmetics we just bump the minimum microversion in Nova
16:20:13 <ildikov> I guess
16:20:34 <mriedem> 1) i wouldn't do the cosmetics personally
16:20:37 <smcginnis> I'm a little confused on that now though. Do we need a bump?
16:20:39 <mriedem> 2) nova doesn't care about the cosmetics
16:20:40 <johnthetubaguy> mriedem: oh, that sounds like a bug, we shouldn't detach the volume for sure, but I thought we should terminate... ?
16:20:51 <mriedem> smcginnis: if you change a key in a request body, you need a microversion change
16:21:02 <tommylikehu> request and response
16:21:05 <johnthetubaguy> FWIW, we haven't bothered doing the cosmetic changes in Nova
16:21:06 <mriedem> johnthetubaguy: i don't see where we terminate on shelve offload
16:21:07 <smcginnis> OK, so Pike Nova should be able to work with Ocata Cinder with these changes, correct?
16:21:14 <mriedem> smcginnis: yes
16:21:20 <smcginnis> That was my concern.
16:21:24 <johnthetubaguy> mriedem: your probably correct, just agreed it sounds wrong
16:21:34 <mriedem> smcginnis: so you guys understand on the cinder side,
16:21:40 <mriedem> if you change a request or response payload,
16:21:43 <mriedem> it's a microversion bump right?
16:21:54 <smcginnis> Yes, definitely.
16:21:57 <johnthetubaguy> right, and the old microversion doesn't change
16:22:08 <ildikov> personally I'm less enthusiastic about the change if it causes too much headache
16:22:12 <mriedem> ok, because i'm a bit worried by the fact that cinder doesn't using jsonschema validation for requests
16:22:20 <ildikov> I get the microversion part
16:22:28 <mriedem> and i don't know if tempest is validating your microversioned responses with jsonschema yet
16:22:43 <smcginnis> mriedem: I don't believe os.
16:22:45 <smcginnis> *so
16:23:01 <mriedem> really kind of need a tight jsonschema validation on request and response when you start supporting microversions
16:23:09 <mriedem> because checks in python code aren't going to catc hstuff
16:23:31 <smcginnis> mriedem: Right, seems like the only way to validate nothing slips through.
16:24:51 <mriedem> https://review.openstack.org/#/c/454436/
16:24:55 <mriedem> you guys don't require a spec for api changes?
16:25:19 <tommylikehu> ...
16:25:33 <tommylikehu> releasenote is required
16:26:04 <mriedem> release notes are fine, from a process standpoint we've found that we need a spec for api changes
16:26:09 <smcginnis> Not always. Usually only need a spec if it's a big enough change that we want to make sure everyone understands what's being done.
16:26:41 <mriedem> ok, that's fine, it's your world - fwiw i've -1ed that proposal since i don't see the value
16:27:08 <tommylikehu> I am ok with that
16:27:14 <ildikov> fair enough
16:34:28 <stvnoyes> i'm at oracle and they're very interested in this for RAC.  what is the expectation for how much of this is going to be completed for Pike?
16:35:11 <ildikov> but as that will be a separate patch anyhow we can deal with that on the review, I would guess we don't want the volume connected to the instance as a principle
16:35:26 <ildikov> stvnoyes: we would love to get the base functionality working
16:35:35 <mriedem> stvnoyes: which thing specifically?
16:35:38 <stvnoyes> and for RAC, we can probably get away with not worrying about migration/swap/bfv
16:35:41 <mriedem> nova using the new attach/detach APIs?
16:35:44 <ildikov> stvnoyes: we need to deal with a few things for multi-attach still
16:35:45 <mriedem> or volume multiattach?
16:36:08 <stvnoyes> twe'd need multi-attach working for at least the zfssa
16:36:24 <mriedem> i expect we'll get the nova using the new api flow with cinder in pike
16:36:30 <mriedem> i don't know about multiattach support honestly
16:36:38 <mriedem> depends on the progress made
16:36:54 <ildikov> I don't think we will get to that if I would need to be brutally honest
16:37:06 <ildikov> not with the current progress at least
16:37:47 <jungleboyj> We have been working toward this for so long that at least having the API improvements is very encouraging.
16:37:48 <ildikov> the more hands on the keyboard the better
16:37:57 <ildikov> we could definitely use more atm
16:38:25 <stvnoyes> if would you feel more confident if it vm's with ma disks could not be migrated, disks couldn't be swapped, bfv not supported. that might simplify things for pike for a first implementation
16:38:51 <mriedem> yeah we'll have to see
16:39:01 <mriedem> first step is using the new apis,
16:39:09 <mriedem> because once we attach with the new style, we have to detach with the new style everywhere
16:39:14 <stvnoyes> i am (slowly) coming up to speed on nova and we have someone else joining us, and hopefully another person in a few more weeks.
16:39:29 <stvnoyes> so we should be able to help
16:39:42 <mriedem> and once you support multiattach (not bfv) then you need to consider what that means for those other operations
16:39:50 <ildikov> stvnoyes: +1, thanks!
16:39:56 <mriedem> unless you just completely punt and fail saying it's not supported
16:40:06 <mriedem> anyway, that's a ways down the road
16:40:30 <stvnoyes> i was thinking that you would throw an api exception if you try to migrate an instance with a ma disk for example
16:40:43 <mriedem> maybe, but we're nowhere near that point yet
16:40:50 <mriedem> so let's not rathole on it :)
16:40:50 <stvnoyes> kk understood
16:40:59 <mriedem> we already did that about a year ago
16:41:20 <mriedem> ildikov: ok done for the meeting?
16:41:42 <ildikov> mriedem: I think we are
16:42:28 <ildikov> stvnoyes: we can catch up after the meeting if you need more info or anytime if you have further questions
16:42:36 <ildikov> stvnoyes: feel free to ping me
16:42:44 <stvnoyes> ok thanks!
16:43:08 <ildikov> is there anything else from anyone for today?
16:44:10 <ildikov> ok I take it as a no
16:44:31 <ildikov> as a summary I will split the detach patch to smaller ones and let's try to merge what we can from it
16:44:38 <ildikov> see you all next week!
16:44:44 <ildikov> thanks for today
16:44:51 <ildikov> #endmeeting