17:01:21 <ildikov> #startmeeting cinder-nova-api-changes
17:01:22 <openstack> Meeting started Mon Jan 23 17:01:21 2017 UTC and is due to finish in 60 minutes.  The chair is ildikov. Information about MeetBot at http://wiki.debian.org/MeetBot.
17:01:23 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:01:26 <openstack> The meeting name has been set to 'cinder_nova_api_changes'
17:01:31 <ildikov> scottda 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:01:41 <hemna> tough
17:01:44 <hemna> heh
17:01:46 <hemna> tough
17:01:55 <ildikov> hemna: morning :)
17:02:05 <hemna> fwiw, I'm on a work call now too.
17:02:13 <ildikov> hemna: I know
17:02:26 <ildikov> hemna: would this slot work for you on any other day than Monday?
17:02:29 <scottda> hi.
17:02:33 <scottda> doubled booked as well
17:02:56 <hemna> I think so, it seems to change week to week for us
17:02:57 <ildikov> hemna: I checked the calendar last week and it seemed we have at least one channel available for this slot whole week
17:03:05 * patrickeast is sorta here too
17:03:10 <diablo_rojo> Hello
17:03:30 * breitz breitz is here (sorta too - double booked)
17:03:43 <ildikov> hemna: hmm, I cannot handle random collisions :) or is there any slot that would surely work? :)
17:04:02 <stvnoyes> steve here...
17:04:30 <hemna> I think this is fine for me for now
17:04:42 <ildikov> ok then I will send out a mail with a Doodle poll
17:04:43 <hemna> I'll just use both halves of my brain, then sleep the rest of the day.
17:05:01 <ildikov> same slot on some other day of the week will hopefully work
17:05:14 <ildikov> hemna: it would work for me :)
17:06:00 <ildikov> breitz: hey, welcome to the club of double booked people :)
17:06:07 <breitz> thanks!
17:06:30 <ildikov> we don't have Matt today, he's on vacation
17:06:44 <ildikov> I couldn't reach johnthetubaguy on the Nova channel earlier either
17:06:55 <ildikov> so it seems like a Nova-less meeting for today
17:07:14 <scottda> Good for him. ildikov , please make note of that NOva patch....
17:07:23 <ildikov> I updated the etherpad we have with the latest news
17:07:25 <ildikov> #link https://etherpad.openstack.org/p/cinder-nova-api-changes
17:07:57 <scottda> #link https://review.openstack.org/#/c/422078/
17:08:13 <scottda> That should be easy to land
17:08:26 <scottda> And needed, regardless of cinder-nova api changes
17:08:26 <ildikov> we need to make Nova to work with Cinder v3, fix for that is ^^
17:08:59 <ildikov> without scottda's patch Nova cannot handle the v3 endpoint thanks to some filtering
17:09:12 <scottda> ildikov: What changes have you made to Nova to test new cinder apis?
17:09:20 <scottda> ildikov: Or do you just use the cinderclient?
17:09:44 <ildikov> scottda: I used it with the client, I hardcoded the version to 3.27 in Nova to use that one
17:10:02 <scottda> ildikov: But what does nova do to make the new Cinder api calls?
17:10:03 <ildikov> scottda: also used your change and set the catalog_info in the nova.conf to point to v3
17:10:16 <ildikov> scottda: it's hardcoded there as well for now
17:10:25 <scottda> ok. Can you share that?
17:10:42 <ildikov> scottda: check here: https://review.openstack.org/#/c/330285/13/nova/volume/cinder.py
17:10:46 <ildikov> scottda: line 533
17:11:27 <scottda> ildikov: OK, so you are using jgriffith 's patches. Thanks.
17:12:00 <ildikov> scottda: so we need to figure out how to negotiate with the client/server on avaliable versions and then I can point this patch to depend on yours or add the changes here
17:12:06 * johnthetubaguy wonders in late
17:12:10 <ildikov> scottda: yep, fixed a few things
17:12:14 <ildikov> johnthetubaguy: hi :)
17:12:21 <ildikov> johnthetubaguy: thanks for joining
17:12:30 <ildikov> then announcements quickly
17:12:46 <ildikov> #info we have the new attach/detach flow patch merged for Cinder
17:13:00 <ildikov> #link https://review.openstack.org/#/c/414632/
17:13:04 <johnthetubaguy> ah, sweet
17:13:10 <ildikov> johnthetubaguy: so this is how it looks now^^
17:13:16 <scottda> ildikov: Sure, I need to work on details with nova folks for https://review.openstack.org/420201
17:13:29 <ildikov> #info with local testing nova volume-attach works with the new Cinder API's
17:13:46 <ildikov> #link https://review.openstack.org/#/c/330285/
17:14:05 <ildikov> I know the tests are failing that's not the first priority for the PoC now :)
17:14:18 <ildikov> thanks for all the reviews on the Cinder patch from everyone!
17:14:30 <scottda> ildikov: What are we thinking is left for Ocata?
17:15:07 <ildikov> scottda: right, we need to figure out how to make Matt's preferred flow work
17:15:18 <ildikov> scottda: I would say testing and fix whatever issues we find
17:15:22 <johnthetubaguy> whats the chances of getting the functional tests passing on that POC? do we know if there are big issues in there?
17:15:36 <ildikov> scottda: johnthetubaguy we also need ot look into the detach flow to work with Nova
17:15:51 <johnthetubaguy> ah, the detach flow might be the issue for those tests then
17:16:04 <ildikov> scottda: and also to figure out how to support multi-attach with the new flow as it does not care about that currently
17:16:20 <scottda> #link https://review.openstack.org/420201
17:16:28 <scottda> johnthetubaguy: Please look at that ^^
17:16:47 <scottda> I'm wondering where to put a hardcoded version of cinder/cinderclient that Nova wishes to use
17:16:56 <scottda> Then I can update that patch
17:17:46 <johnthetubaguy> OK, so step one is move nova to Cinder v3 API I guess
17:17:48 <ildikov> johnthetubaguy: the PoC fails with one minor issue I updated some function headers and it's not consistent now
17:18:20 <ildikov> johnthetubaguy: the other thing that's failing is that I thought to keep the old tests on the old flow and there's one test I couldn't figure out how to mock the version properly... :S
17:18:38 <ildikov> johnthetubaguy: if we want to keep the old tests until we support the old flow, I might help with that
17:18:50 <jgriffith> It must be Monday
17:19:15 <johnthetubaguy> scottda: I think just hard coding the version in a constant is fine
17:19:16 <ildikov> jgriffith: your instinct work well :)
17:19:28 <ildikov> *instincts
17:19:39 <scottda> johnthetubaguy: OK, I'll put that in nova/volume/cinder.py
17:19:56 <johnthetubaguy> scottda: yeah, I think thats fine.
17:20:17 <ildikov> johnthetubaguy: hardcoding the one that Nova would wish to use or what Cinder has to offer or both? :)
17:20:19 <jgriffith> ildikov scottda now that the Cinder side is fairly stable I can rework the Nova patch with ildikov to get the POC of it working again
17:20:40 <ildikov> jgriffith: I made nova volume-attach work yesterday :)
17:20:44 <jgriffith> I am not familiar with the reference about the "preferred flow" from Matt R though?
17:21:01 <johnthetubaguy> so, microversions mean you have to keep all old versions, so what matters is what versions work with Nova
17:21:03 <ildikov> jgriffith: or well, no failures in the logs and I think I saw some iSCSI stuff there too happening :)
17:21:04 <scottda> ildikov: Hard code the cinder version that Nova wants to use...
17:21:16 <johnthetubaguy> for now, I guess we just request the base version?
17:21:33 <scottda> ildikov: Then get the cinderclient MAX_VERSION and cinder server version dynamically
17:21:45 <scottda> Then get the MIN of those 3
17:21:45 <ildikov> scottda: in my head it looks like we have an if where we want to use stuff that needs a microversion and if we have that then go and if we don't then use the old stuff
17:21:51 <ildikov> scottda: we need that anyhow
17:21:53 <jgriffith> oh, ya'll are talking about the micro-version madness; I can't help there
17:22:15 <ildikov> jgriffith: we just can't get out of that loop...
17:22:18 <jgriffith> well, I can help
17:22:21 <johnthetubaguy> so from the nova side it should be fairly simple
17:22:25 <ildikov> jgriffith: I blame Monday...
17:22:36 <johnthetubaguy> there is a requirement to use the new flow, else we do the old flow
17:22:42 <jgriffith> Look, I'll just hack a check like a had before in nova/volume/cinder.py
17:22:56 <jgriffith> Call the client, check something like "v2_supported"
17:23:11 <jgriffith> if it raises or the API call is not found, then it doesn't support it
17:23:19 <jgriffith> else use the new code
17:23:19 <johnthetubaguy> so if new-flow-micoversion-7 supported, use_new_flow = True or something?
17:23:28 <jgriffith> johnthetubaguy yes
17:23:38 <jgriffith> johnthetubaguy it's not elegant but it works
17:23:41 <johnthetubaguy> surely v3 support the old flow though right?
17:23:46 <scottda> johnthetubaguy: yes
17:23:51 <ildikov> jgriffith: you have that check in the PoC decide that it's legacy or new flow
17:23:51 <jgriffith> johnthetubaguy yes for sure
17:23:53 <scottda> old flow never goes away
17:23:57 <ildikov> jgriffith: that works pretty well
17:24:01 <johnthetubaguy> right, that all makes sense
17:24:05 <jgriffith> scottda umm, not true!
17:24:08 <ildikov> we just need the info from the client that what version it can offer
17:24:13 <johnthetubaguy> so scottda I think there are two changes
17:24:24 <jgriffith> scottda once we get this in Nova old flow gets deprecated and yes it most certainly does go away
17:24:27 <scottda> jgriffith: Well, there are those who would yell if we got rid of old apis
17:24:39 <scottda> jgriffith: OK, well that's not my battle to fight.
17:24:41 <ildikov> scottda: with proper deprecation it should be able to go away IMHO
17:24:43 <jgriffith> scottda I fundamentally disagree
17:24:44 <johnthetubaguy> jgriffith: but thats how microversions works, all your old API live for ever..., well until you raise the min_support API version I guess
17:24:54 <johnthetubaguy> anyways, thats a different debate
17:24:57 <jgriffith> johnthetubaguy and that's one of the flaws in microversions
17:24:59 <jgriffith> but ok
17:25:05 <jgriffith> I'm out of it
17:25:17 <johnthetubaguy> jgriffith: we call it a feature, but yeah, depends what camp you are in
17:25:24 <jgriffith> johnthetubaguy :)
17:25:57 <johnthetubaguy> scottda: so maybe you have two changes there, support v3, and decide when its safe to use the new flow?
17:26:06 <scottda> johnthetubaguy: yes
17:26:13 <scottda> they are 2 patch sets
17:26:15 <johnthetubaguy> cools
17:26:16 <jgriffith> johnthetubaguy ildikov scottda I said this before, and I'll say it again; if there's going to be a requirement that no flow ever gets taken away then revert the change that I implemented and we go back to reworking the existing API's
17:26:25 <jgriffith> becuase keeping both in there is madness
17:26:36 <scottda> jgriffith: I am happy to deprecate stuff.
17:26:40 <jgriffith> and there's no way you're going to be able to continue to support, test and maintain both
17:26:45 <scottda> jgriffith: I won't argue against it.
17:27:05 <ildikov> johnthetubaguy: we have checks like this in line 2086 here: https://review.openstack.org/#/c/330285/13/nova/compute/api.py
17:27:16 <ildikov> johnthetubaguy: in the PoC
17:27:49 <ildikov> jgriffith: I'm a deprecation fan for sure!
17:27:51 <johnthetubaguy> ildikov: yeah, makes sense
17:28:39 <ildikov> johnthetubaguy: scottda so we only need to replace that hardcoded number with the version the client gives back and if it's >= 3.27 than we're good...
17:28:55 <jgriffith> ildikov +1
17:29:13 <ildikov> jgriffith: maintaining both would be a real nightmare and I don't think we want that, I surely don't
17:29:15 <scottda> ildikov: Sure.
17:29:55 <ildikov> scottda: so if you can make the negotiation work in your patch for a version that would be great and we can use that as a base for the PoC
17:30:18 <ildikov> jgriffith: so just get rid of the old flow and blame me, I can take it :)
17:30:19 <scottda> ildikov: Yes. And we need to decide how to get that info. A global? a getter?
17:31:08 <scottda> I can code it up any way we want
17:31:13 <johnthetubaguy> SUPPORTS_NEW_FLOW=None and if its None, you set that before you return the client?
17:31:38 <ildikov> scottda: honestly I personally don't have a strong preference on that at this point
17:31:52 <scottda> Sure. I'll set a Global for now. I'll update that patch set today.
17:32:00 <johnthetubaguy> my preference is for simple (but also cached)
17:32:01 <ildikov> johnthetubaguy: let's stick with a version number, people will get confused if we have a variable like that IMHO
17:32:19 <johnthetubaguy> but then you need to spread the version number everywhere?
17:32:58 <johnthetubaguy> its hard to know without seeing it all in gerrit, if I am honest
17:33:06 <johnthetubaguy> its probably there, I just haven't read it
17:33:12 <ildikov> you need spread either this info or the version number everywhere anyway :)
17:33:20 <jgriffith> johnthetubaguy yeah, I like the idea of just checking the version number on init and storing as part of the cinder object in Nova
17:33:31 <ildikov> and I think the version number is crystal clear :)
17:33:33 <jgriffith> ez/pz and cached in memory
17:33:49 <scottda> ildikov: Version number spread throughout nova won't be clear to everyone...
17:33:50 <johnthetubaguy> yeah, the cached in memory bit is I think worth it
17:33:59 <johnthetubaguy> beyond that, just keep it simple
17:34:33 <johnthetubaguy> so going back to this patch:
17:34:35 <ildikov> scottda: but if we will have something new in the future that's connected to a particular higher version number we will have a zillion boolean variables for all if we follow this model
17:34:39 <johnthetubaguy> https://review.openstack.org/#/c/420201
17:35:01 <jgriffith> ildikov oh dear, please no :(
17:35:06 <johnthetubaguy> ildikov: I think the idea is we evenutally delete the ifs, and get back to one path though
17:35:20 <jgriffith> ildikov I'd hope the interface won't change again so there wouldn't be a risk of that
17:35:29 <jgriffith> johnthetubaguy +1
17:35:35 <ildikov> johnthetubaguy: sure, if this boolean suggestion brings us closer to that I'm all in!
17:35:51 <johnthetubaguy> looking at https://review.openstack.org/#/c/420201
17:35:58 <johnthetubaguy> there is a chunk in there about the microversion
17:36:03 <ildikov> johnthetubaguy, jgriffith: I'm always paranoid on Mondays, please forgive me :)
17:36:08 <johnthetubaguy> its tempting to leave that to a later patches
17:36:22 <jgriffith> LOL
17:36:27 <jgriffith> I"m always paranoid in general :)
17:36:41 <johnthetubaguy> ildikov: a good dose of paranoia can be helpful
17:36:45 <scottda> johnthetubaguy: Well, we can get the cinder api version from the server before we instantiate the cinderclient
17:36:46 <johnthetubaguy> what john said
17:36:55 <ildikov> jgriffith: maybe not a bad strategy around here :)
17:37:23 <johnthetubaguy> scottda: ah, I see what you are wanting now
17:37:42 <johnthetubaguy> scottda: maybe just call it MAX_SUPPORT_CINDER_API_VERSION?
17:37:57 <scottda> johnthetubaguy: That patch needs an update...to use Nova's idea of which max cinder version it can use
17:38:15 <johnthetubaguy> so I think there are two bits here
17:38:17 <scottda> johnthetubaguy: sure, I'll call it that. And set it before nova instantiates cinderclient
17:38:23 <johnthetubaguy> whats the max version
17:38:31 <johnthetubaguy> is that high enough for us to use the new flow?
17:38:58 <johnthetubaguy> most calls can just default to not specifying a micro version and using the base version, I guess?
17:39:09 <scottda> johnthetubaguy: yes, but 3 bits: max cinder server version, max cinderclient version, nova's desired cinder version to use new stuff
17:39:20 <johnthetubaguy> cinderclient version?
17:39:23 <ildikov> johnthetubaguy: base version?
17:39:50 <johnthetubaguy> ildikov: the version you get without requesting a micro version, its possible you don't have that
17:39:51 <scottda> johnthetubaguy: Maybe cinder server has new APIs, but you haven't updated your client
17:40:01 <johnthetubaguy> scottda: yeah, gotcha, just being slow
17:40:02 <ildikov> johnthetubaguy: ah, ok
17:40:06 <scottda> johnthetubaguy: Trivial case, but possible. So needs checking.
17:40:25 <johnthetubaguy> does the client tell us what is the max version it and the server speaks?
17:40:34 <johnthetubaguy> that sounds like the one we want to record
17:40:34 <scottda> It could...
17:40:35 <ildikov> scottda: I guess the client can return the lower from those two versions or smth like?
17:40:57 <scottda> Right now, client has MAX_VERSION
17:41:14 <ildikov> scottda: as if the client does not have the code snippet Nova tries to call than it doesn't matter what the server has
17:41:23 <scottda> And I have patch in flight for static method that takes endpoint url and gets version info from tthe server..
17:41:42 <scottda> Client could get MIN of those 2 , and return in a method. Or nova could do the work.
17:42:01 <scottda> Either way
17:42:28 <johnthetubaguy> oh wait, we pass the microversion into the client constructor?
17:42:28 <ildikov> I don't think Nova should know that
17:42:39 <scottda> I'll put up a cinderclient patch to return the MIN of (cinderclient_MAX_VERSION, cinder_server_VERSION)
17:42:47 <scottda> johnthetubaguy: Yes
17:43:05 <ildikov> johnthetubaguy: currently we do, I would be happier to just get it TBH, but I can easily miss smth here
17:43:08 <scottda> johnthetubaguy: We did this prior to microversions
17:43:25 <johnthetubaguy> scottda: I was kinda expecting to just pass in "3"
17:43:31 <scottda> It just used to me major version (1|2|3) and now can take minor version (microversion)
17:43:34 * ildikov is not a microversion specialist on any level in any parallel universe...
17:43:45 <scottda> microversion == minor version
17:43:51 <scottda> ildikov: That's all you need to know
17:43:56 <scottda> APIs are versioned.
17:43:58 <johnthetubaguy> I can't remember what we do for nova right now
17:44:02 <scottda> So you know what you need, and are getting
17:44:11 <johnthetubaguy> novaclient I mean
17:45:51 <johnthetubaguy> so for this first patch, should we just pass 3.0?
17:46:22 <johnthetubaguy> if version == "3"
17:46:29 <scottda> no
17:46:41 <scottda> johnthetubaguy: Because this is a much higher version
17:46:56 <scottda> johnthetubaguy: cinder version 3.0 does not have the new APIs for attach/detach
17:46:59 <johnthetubaguy> so first version is just calling the old flow, on v2.0
17:47:03 <johnthetubaguy> I mean v3.0
17:47:04 <scottda> yes
17:47:08 <johnthetubaguy> oh.....
17:47:11 <scottda> v3.0 == v2
17:47:20 <johnthetubaguy> yeah
17:47:31 <johnthetubaguy> so thats what we want for the first patch right?
17:47:47 <johnthetubaguy> just use the old APIs on the new v3 end point?
17:47:48 <scottda> "that" is what?
17:47:57 <scottda> ahh..right
17:48:27 <johnthetubaguy> I am thinking pure in isolation enable the use of v3
17:48:54 <johnthetubaguy> then second step is determine if can use new flow (v3.42 say) vs old flow (v3.0)
17:49:08 <scottda> johnthetubaguy: Yes, as of today, we cannot use v3
17:49:18 <johnthetubaguy> oh dear, now I get you
17:49:25 <scottda> That's why we need https://review.openstack.org/422078
17:49:41 <scottda> That is a crappy bug because of how cinder has 3 things in the keystone catalog...
17:49:52 <scottda> "volume", "volume2", "volume3"
17:50:00 * scottda would like to change that
17:50:03 <ildikov> scottda: oh, that's kinda terrible
17:50:07 <johnthetubaguy> volume4 that just points to the route?
17:50:21 <scottda> we've no volume4 ATM
17:50:29 <scottda> (and hopefully never will)
17:50:40 <johnthetubaguy> yeah, volume4 points to v2 and v3's route
17:50:44 <johnthetubaguy> or something
17:50:52 <ildikov> scottda: although after fixing the context list the 'catalog_info' still needs to be set
17:50:59 <scottda> Well, how we fix it, I'm not sure...
17:51:17 <ildikov> scottda: that's what we need the devstack config for we talked about a few weeks ago
17:51:29 <scottda> I'm a ML post and plans at PTG to talk about the service catalog. Please attend.
17:51:48 <scottda> ildikov: Sorry, which config was that?
17:51:55 <scottda> ildikov: you mean to use v3 endpoint?
17:52:08 <scottda> [cinder] catalog_info = volumev3:cinderv3:publicURL
17:52:11 <ildikov> scottda: yeah
17:52:34 <ildikov> Matt said that some post Devstack config can fix that so we can test v3 in the placement_api job
17:52:38 <scottda> yup, that and my bug fix
17:52:44 <ildikov> yep
17:53:15 <johnthetubaguy> I think I am better understanding the earlier questions around the version number now, which is good
17:54:26 <johnthetubaguy> we totally need to sit down and talk service catalog standardisation again, I remember sdague started some good conversations around that
17:54:29 <ildikov> johnthetubaguy: cool, then this meeting was useful :)
17:54:38 <johnthetubaguy> it was for me
17:55:13 <ildikov> we have 5 minutes left
17:55:31 <ildikov> I guess we're more or less on track regarding what we want to do with the versions
17:56:05 <ildikov> I will send out a Doodle poll to the list trying to find another slot so we don't have that many collisions as we have now
17:56:57 <ildikov> starting from next week I hope we can sort out what we need to make the PTG discussions good and productive
17:57:12 <scottda> ildikov: Yeah. I've a placeholder in cinder etherpad...
17:57:15 <ildikov> also we need to figure out what we need on the Cinder side for the multi-attach bits
17:57:15 <scottda> For meeting with Nova
17:57:18 <ildikov> jgriffith: ^^
17:57:31 <scottda> We need to perhaps schedule a time for that.
17:57:46 <ildikov> scottda: I think Matt added a similar placeholder to the Nova etherpad too, so we should be covered from this perspective
17:58:30 <ildikov> scottda: yeah, I'll try to ping smcginnis and mriedem about timing this when both are available
17:59:59 <ildikov> ok, anything else for today? :)
18:00:34 <ildikov> all right, thanks everyone!
18:00:55 <ildikov> see you all next week at this or at another slot :)
18:00:59 <ildikov> #endmeeting