Thursday, 2016-06-02

*** amrith is now known as _amrith_00:05
*** sdake_ is now known as sdake00:19
*** beekhof has quit IRC00:42
*** beekhof has joined #openstack-meeting-cp00:48
*** sheel has joined #openstack-meeting-cp01:28
*** Niham has joined #openstack-meeting-cp01:31
*** Niham has quit IRC01:32
*** Niham has joined #openstack-meeting-cp01:33
*** sdake has quit IRC01:49
*** Niham has quit IRC02:10
*** sdake has joined #openstack-meeting-cp02:16
*** sdake has quit IRC02:18
*** itisha has quit IRC02:30
*** tyr_ has quit IRC03:15
*** Niham has joined #openstack-meeting-cp04:09
*** Niham has quit IRC04:10
*** Niham has joined #openstack-meeting-cp04:11
*** Niham has quit IRC04:57
*** Niham has joined #openstack-meeting-cp05:06
*** tyr_ has joined #openstack-meeting-cp05:16
*** tyr_ has quit IRC05:20
*** Niham has quit IRC05:46
*** tyr_ has joined #openstack-meeting-cp06:17
*** tyr_ has quit IRC06:21
*** coolsvap_ has joined #openstack-meeting-cp06:44
*** Niham has joined #openstack-meeting-cp06:51
*** Niham has quit IRC06:57
*** sdake has joined #openstack-meeting-cp06:57
*** coolsvap_ has quit IRC07:13
*** coolsvap_ has joined #openstack-meeting-cp07:15
*** tyr_ has joined #openstack-meeting-cp07:18
*** tyr_ has quit IRC07:22
*** sdake has quit IRC07:39
*** coolsvap_ has quit IRC07:59
*** tyr_ has joined #openstack-meeting-cp08:18
*** Niham has joined #openstack-meeting-cp08:19
*** tyr_ has quit IRC08:23
*** markvoelker has quit IRC08:29
*** markvoelker has joined #openstack-meeting-cp08:30
*** sdake has joined #openstack-meeting-cp09:13
*** tyr_ has joined #openstack-meeting-cp09:19
*** tyr_ has quit IRC09:24
*** _amrith_ is now known as amrith09:37
*** sdake has quit IRC09:40
*** tyr_ has joined #openstack-meeting-cp10:21
*** Niham has quit IRC10:22
*** Niham has joined #openstack-meeting-cp10:24
*** tyr_ has quit IRC10:25
*** amrith is now known as _amrith_10:40
*** sdague has joined #openstack-meeting-cp10:46
*** tyr_ has joined #openstack-meeting-cp11:22
*** tyr_ has quit IRC11:26
*** _amrith_ is now known as amrith11:52
*** tyr_ has joined #openstack-meeting-cp12:23
*** tyr_ has quit IRC12:27
*** Niham has quit IRC12:45
*** amrith is now known as _amrith_13:00
*** _amrith_ is now known as amrith13:09
*** xyang1 has joined #openstack-meeting-cp13:31
*** Niham has joined #openstack-meeting-cp13:37
*** Niham has quit IRC13:38
*** bswartz has quit IRC14:27
*** david-lyle has joined #openstack-meeting-cp14:44
*** bswartz has joined #openstack-meeting-cp14:51
*** amrith is now known as _amrith_15:02
*** ildikov has quit IRC15:07
*** patrickeast has quit IRC15:08
*** _amrith_ is now known as amrith15:08
*** ildikov has joined #openstack-meeting-cp15:09
*** patrickeast has joined #openstack-meeting-cp15:09
*** amrith is now known as _amrith_15:09
*** _amrith_ is now known as amrith15:14
*** hemnafk is now known as hemna15:23
*** breton_ is now known as breton16:13
*** david-lyle has quit IRC16:24
*** sdake has joined #openstack-meeting-cp16:44
*** mriedem has joined #openstack-meeting-cp17:00
ildikov#startmeeting cinder-nova-api-changes17:00
openstackMeeting started Thu Jun  2 17:00:22 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
openstackUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.17:00
*** openstack changes topic to " (Meeting topic: cinder-nova-api-changes)"17:00
openstackThe meeting name has been set to 'cinder_nova_api_changes'17:00
ildikovscottda DuncanT ameade cFouts johnthetubaguy jaypipes takashin alaski e0ne jgriffith tbarron andrearosa hemna erlon mriedem gouthamr ebalduf patrickeast smcginnis diablo_rojo gsilvis17:00
scottdahi17:00
mriedemo/17:00
jgriffitho/17:00
*** andrearosa has joined #openstack-meeting-cp17:01
patrickeasthey17:01
andrearosao/17:01
ildikovhi everyone17:01
hemnahey17:01
ildikovwe have our etherpad #link https://etherpad.openstack.org/p/cinder-nova-api-changes more or less up to date17:02
hemnashameless plug....I'm still looking for a job. :P17:02
hemnaso I'm able to get BFV 'working'17:03
hemnawith reserve_volume17:03
ildikovhemna should I add it to the etherpad as well?17:03
ildikov:)17:03
ildikovhemna: that's good news!17:03
hemnabut I'm having an issue reporting the an exception back up the stack when the volume is already in use.17:03
hemnaI'm working on that17:03
hemnaInvalidInput(u'Invalid input received: Invalid volume: Volume status must be available to reserve.17:04
ildikovhemna: did you remove the second check_attach?17:04
hemnanot sure why I'm getting recursive messages as the exception message at this point17:04
hemnabut at least I have the http response code correct17:04
hemnaso it's not a nova api change (other than a message change)17:04
hemnaildikov, kinda17:04
hemnaI just blindly removed the check_attach call17:05
hemna:P17:05
mriedemhemna: that won't work17:05
jgriffithhemna: is there a bug for what you're working on?17:05
hemnamriedem, ok17:05
mriedemhemna: when you get to the compute in BFV17:05
mriedemit's a cast by then17:05
mriedemso you've lost the api response to the user17:05
ildikovjgriffith: there are a few race condition type of bug reports, this should help in those17:05
hemna?17:05
mriedemso you'd basically fail and the instance would go into error state17:05
hemnaI'm not following17:05
hemnathe API response isn't different to the user as it is now17:05
hemnaother than the text of the message17:06
jgriffithildikov: well, I'm looking specifically to understand what is being discussed without taking up meeting time :)17:06
mriedemi guess i'm confused on where you're calling reserve_volume in that flow17:06
mriedemhemna: from the API or from the compute?17:06
hemnamriedem, API17:06
ildikovhemna: you have the reserve call, where the first check_attach was in validate_bdm?17:06
hemnaI'll post up the patch today17:06
mriedemhemna: oh, in that case, nevermind17:06
hemnaI'm replacing the existing check_attach call in the API17:06
hemnawith reserve_volume17:06
mriedemthere are some wrappers on the reserve method in the nova.volume.cinder api method17:06
mriedemto handle errors17:07
ildikovjgriffith: with hemna we're aiming at removing the check_attach calls from Nova as a first step on our journey to refactor17:07
mriedemmaybe something missing in those error decorators17:07
*** coolsvap_ has joined #openstack-meeting-cp17:07
hemnaand detecting the case when the volume is 'in-use' and raising the same exception as it does today, just with a different message.17:07
hemnaI can put up a gist17:07
hemnait's total hack code at this point17:07
ildikovif you replaced that API check_attach that should be the right direction17:08
jgriffithildikov: ok... just curious as to why17:08
hemnahttps://gist.github.com/WaltHP/12d86a9edf7c21b50d2aff443056dfc617:08
hemnathat's on top of my existing patch17:08
ildikovjgriffith: the check_attach call is unnecessary for Nova, it's kind of a left over from old times17:08
ildikovjgriffith: but it's a change that can be handled in a reasonable timeframe, therefore it seems worth starting with17:09
hemnajgriffith, the existing nova/volume/cinder.py check_attach looks at the internal state of the cinder volume17:09
jgriffithildikov: okie-dokie17:09
hemnatrying to eliminate that17:09
hemnaand just rely on Cinder to do the check that already exists in os-reserve17:09
hemnait's the same check17:09
jgriffithildikov: do we have an agenda today or is it open-discussion?17:09
ildikovjgriffith: so basically we're trying the baby steps approach to have some progress, while discussing more radical changes, like the new API calls you're working on17:10
hemnaok I'm done if we want to move on17:10
hemnaI just wanted to give a status17:10
mriedemfor the multiattach spec,17:10
mriedemjohnthetubaguy:  is +2 on it,17:10
mriedemi got back to the world <48 hours ago17:11
ildikovjgriffith: the agenda is to discuss the ongoing items, like the one hemna brought up, bring up your work item, plus get the multi-attach spec in Nova accepted by mriedem17:11
mriedemi will get through the spec today17:11
*** coolsvap_ has quit IRC17:11
jgriffithlink 2 spec?17:11
mriedemhttps://review.openstack.org/#/c/304681/17:11
jgriffithmriedem: gracias17:11
ildikovhemna: cool, tnx, it looks good17:12
ildikovmriedem: the spec is aiming at capturing what we need to deal with and points at the etherpad as well17:13
ildikovmriedem: I tried not to add too many details about the dependencies there17:13
ildikovif nothing for the spec, I think we can move to what jgriffith is working17:14
jgriffithildikov: ok, I guess I'll just chat about some things17:16
ildikovhere is the link to the review: https://review.openstack.org/#/c/323571/17:16
jgriffithI have 3 different approaches going on currently:17:16
jgriffith1. Just massage the existing flow and calls:  https://review.openstack.org/#/c/32072117:16
jgriffithUgly hacky stuff17:16
jgriffith2. Introduce some new calls:  https://review.openstack.org/#/c/323571/ but leverage all the old stuff (helps with unit tests and back compat)17:17
jgriffith3. Write new stuff:  https://github.com/j-griffith/cinder/commit/4feabd244e755dd02f930c1efa2c4d59f89b0c2e17:17
jgriffith3 is my favorite :)  I'm also working on Nova changes as POC to go with it.  Basically want to have a full working solution with multi-attach using option 3 in another week17:18
jgriffithI can work on all, none or one of these17:19
jgriffithobviously they're not complete, nor functional as they are right now.  Like the copy/paste of _get_connection_count etc17:20
ildikov2 contains parts from 3, right?17:20
ildikovor at least the concept of the new calls is going to that direction, IIUC17:20
jgriffithildikov: yes... they're all related17:20
ildikovok, cool17:20
jgriffithildikov: but the more time I spend looking at the attach/detach code in Cinder, the more I realize it's become a bit of a train wreck17:21
* hemna is a fan of 317:21
ildikovI like that we're trying to simplify the interaction between Cinder and Nova as much as possible so having one call as opposed to three or more17:21
scottdajgriffith: Will you be pushing #3 up to gerrit ?17:21
jgriffithand taking the opportunity to do something like 3 (while it may be scary to some) eliminates the issues folks have with race-conditions etc17:21
jgriffithscottda: nope, I refuse :)17:21
jgriffithscottda: of course17:21
scottdahaha17:21
hemna:)17:22
jgriffithscottda: but it's got a ways to go before being even WIP status in Gerrit IMO17:22
mriedemfrom a nova perspective, they are all bound by microversions so not sure i have an opinion17:22
scottdacool17:22
jgriffithand I didn't want to bombard everybody with yet another gerrit patch yet17:22
ildikovjgriffith: sure, that I completely get, I'm wondering about the amount of changes in NOva and the timeframe we would need to introduce them17:22
mriedemand really would start as only calling those if nova is given a multiattach volume (i think)17:22
jgriffithmriedem: so that's a possibility if preferred17:23
jgriffithmriedem: I do plan to provide a nova patch to go with it FWIW17:23
jgriffithmriedem: although it will surely not be up to standards with unit tests and some of the versioning stuff17:23
jgriffithmriedem: I at least wanted to take a swag at a proof of concept17:23
mriedemsure17:24
mriedemthat's fine17:24
jgriffithmriedem: my question to you though would be... out of the choices, what has a chance of landing in N17:24
mriedemwell those 3 are all cinder changes17:24
ildikovwith smaller or larger Nova impacts17:25
jgriffithmriedem: hehe.. yeah, but they will all require something on the Nova side to utilize them17:25
mriedemso #1 is existing APIs but new behavior, right?17:25
jgriffithmriedem: in other words, I want to finish and release multi-attach in N and get past all of this17:25
ildikov#3 is the rethinking of the attach concept17:25
patrickeastjgriffith: whats the scale difference as far as nova changes required between 1, 2 and 3?17:25
jgriffithmriedem: pretty much, yes... but it's super ugly in terms of "optional" args/params (I think it's more trouble than it's worth)17:26
mriedemso, a thing that might suck on the nova side is if volume['multiattach'], and we have to call a different API - however, that can all be abstracted17:26
jgriffithmriedem: I'd suggest we don't do that if possible17:26
mriedemyeah17:26
mriedemso i'm not opposed to new apis that are better17:26
jgriffithmriedem: I've been looking at just replacing the nova calls with the new calls in option-317:26
ildikovmriedem: in my understanding these new API calls are not for multiattach17:27
jgriffithpatrickeast: they're in ascending order in terms of amount of change in Nova17:27
mriedeme.g. cinder provides new api in microversion 2.6 which is the preferred way to do this, and consider attach/detach deprecated at that point17:27
mriedemso clients would start moving17:27
jgriffithmriedem: ok... that's certainly fair17:27
patrickeastjgriffith: yea, but like... from 2 to 3 are we talking incremental amount of differences... or like 10x?17:27
ildikovmriedem: I mean they would be prepared to handle multiattach as well, but it's about to remove the reserve, initialize_connection, attach chain in my understanding17:27
jgriffithmriedem: I was more concerned that we were just already too late in the cycle for this sort of thing17:27
mriedemmy preference is to take the time to do the right thing for the long-term, whatever that is17:28
jgriffithmriedem: and YES, exactly my plan would be to add the deprecation marker to reserve, initialize_connection and attach etc17:28
patrickeastmriedem: +117:28
mriedemif that's using new better cleaner APIs in cinder, i think we do that regarldess of schedule17:28
jgriffithmriedem: ok, that's the best answer I could hope fo17:28
jgriffithfor17:28
mriedembecause i don't want to shoehorn #1 and introduce more tech debt17:29
hemnaI think the long term thing is option #317:29
jgriffithmriedem: yes, that's the sort of thing that created the mess we have now to begin with17:29
hemnaI'd like to remove the check_attach stuff short term and rely on reserve_volume17:29
jgriffithIt's unfortunate I didn't recognize some of this sooner or as it was happening17:29
hemnathat should make it easier to migrate to #3, since it's all cinder side checks17:29
ildikovmriedem: the regardless of schedule means being able to merge changes in Nova regardless of schedule?17:30
ildikovmriedem: also totally agree on the long term thinking17:30
jgriffithildikov: hehe.. I think it means, "do the right thing even if it takes another cycle"17:30
hemnajgriffith, +117:30
mriedemwhat jgriffith said17:30
ildikovjgriffith: if it's only one...17:30
hemnaildikov, it's never one17:30
ildikovdo we have any incremental choice here?17:31
jgriffithildikov: I tihnk incremental in this case is bad17:31
mriedem+117:31
mriedemthe problem with incremental is you bake in the badness, and then it's hard to move17:31
jgriffithildikov: I also think that #3 is incremental.. because to start it's mostly wrapping existing cinder calls17:31
mriedemnot only technically, but because people don't want to do it17:32
jgriffithmriedem: +117:32
ildikovsure, I'm not saying I'm against the good things, I'm just sometimes invited to customer meetings... :S17:33
hemnaok so anyway, what is the plan then?17:34
hemnaI vote for #317:34
jgriffithI'd propose I finish up POC's of #3 for nova and cinder and post them within the next week17:35
ildikovshould we do a vote?17:35
jgriffithdoes that work?17:35
jgriffithildikov: oh.. yeah... we can vote :)17:35
ildikovjgriffith: what is the aim from the POC?17:35
ildikovjgriffith: is it about whether this works or more about how hard to get it work?17:35
jgriffithildikov: ummm... Proof Of Concept?17:35
jgriffithildikov: yes, see if people vomit all over it, or if it's acceptable and we all pitch in to finish/merge it17:36
mriedempoint is to see what the changes look like and see if there are red flags17:36
mriedemthat make it a non-starter17:36
ildikovjgriffith: I know the term :)17:36
jgriffithmriedem: +117:36
jgriffithildikov: sorry... mriedem 's answer was better17:36
ildikovok, let's see the PoC then17:37
mriedemfwiw if the nova spec is generic enough to know we're playing with new things here,17:38
ildikovjgriffith: is there any work item where either of us could join?17:38
mriedemi'm fine with getting that in today - just need the time to look at it17:38
mriedemildikov: testing probably17:38
ildikovmriedem: the plan was to have it that generic, so I referred to having new Cinder API microversion as we cannot avoid that17:39
jgriffithildikov: Give me til towards end of next week to get patches up.  Then yes, testing17:39
ildikovmriedem: the rest is supposed to be on us17:39
ildikovjgriffith: ok17:39
hemnacoolio17:41
ildikovshould we continue with trying to remove check_attach, etc?17:41
ildikovjust to maintain a plan B if #3 would not work out for any reason17:42
mriedemremoving check_attach still applies for those that aren't at the microversion required to use the new stuff17:43
mriedemso seems worthwhile17:43
hemnaildikov, I think the removal of the check_attach stuff is a bug fix IMHO.17:43
ildikovok, cool17:43
hemnaBFV currently doesn't even call reserve_volume17:43
hemna:(17:43
ildikovyeah, don't even remind me, I checked it zillion times as I was sure I'm not able to debug properly...17:44
ildikovok, do we have anything else for today to discuss?17:46
ildikovso as a quick summary we have the check_attach removal and the #3 prototyping activity ongoing17:47
ildikovthere was also an item about testing Cinder migrate17:48
mriedemyeah are there any updates on that testing?17:48
ildikovalthough for the above two that does not seem to be a direct requirement17:48
mriedemumm17:49
mriedemswap_volume uses check_attach right?17:49
ildikovmriedem: that checks everything before check_attach TBH17:49
mriedemso, it would be good to have some kind of integration test coverage before changing all that, but it could be tested manually too17:49
hemnayah I'd like to have that testing in place17:50
mriedemok, i do seem to remember a bunch of checks in the compute api for swap_volume17:50
ildikovscottda: do you have any news about testing?17:50
scottdaSorry had to run afk...17:50
scottdaI'm making progress.17:50
ildikovmriedem: I tried to refactor that when I touched check_attach for multi-attach, I partially failed due to the order of checks there...17:51
scottdaI need to add plumbing to have cinder tests do Nova create and volume attach.17:51
scottdaHopefully that's not took painful.17:51
mriedemscottda: it should be a scenario test17:52
mriedemnot a tempest api test17:52
scottdaS/took/too17:52
mriedemscenario tests extend a base manager that does a lot of that kind of stuff for you17:52
scottdamriedem: OK. I'll have a look.17:52
mriedemhemna: ildikov: btw, i think i see where BFV with source=volume calls check_attach, but we can talk after the meeting17:54
*** andrearosa has left #openstack-meeting-cp17:54
ildikovmriedem: it calls it two times if I;m not mistaken17:54
mriedemoh nvm, it calls check_attach but not reserve17:55
hemnado_check_attach defaults to true17:55
hemnamriedem, correct17:55
hemnathat's what I want to change17:55
ildikovmriedem: yeah, no reserve17:55
hemnaand in the process eventually remove check_attach completely.17:55
hemnaif possible.17:55
ildikovit should be, having two separate modules maintaining one single state machine is really not ideal17:56
ildikovjust to state the obvious :)17:56
ildikovok, we're almost out of time17:57
ildikovwe have the next meeting on Monday, I think it would be good to briefly sync up and get back to the normal schedule17:57
ildikovany other last minute items for the meeting?17:58
mriedemnope17:59
ildikovok, then thanks everyone, we had a good chat17:59
scottdaThanks17:59
ildikovand talk to you on Monday!17:59
ildikovor earlier on either channel to address implementation details :)18:00
ildikov#endmeeting18:00
*** openstack changes topic to "OpenStack Meetings || https://wiki.openstack.org/wiki/Meetings"18:00
openstackMeeting ended Thu Jun  2 18:00:15 2016 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)18:00
openstackMinutes:        http://eavesdrop.openstack.org/meetings/cinder_nova_api_changes/2016/cinder_nova_api_changes.2016-06-02-17.00.html18:00
openstackMinutes (text): http://eavesdrop.openstack.org/meetings/cinder_nova_api_changes/2016/cinder_nova_api_changes.2016-06-02-17.00.txt18:00
openstackLog:            http://eavesdrop.openstack.org/meetings/cinder_nova_api_changes/2016/cinder_nova_api_changes.2016-06-02-17.00.log.html18:00
*** mriedem has left #openstack-meeting-cp18:00
*** david-lyle has joined #openstack-meeting-cp18:09
*** Guest35638 is now known as cFouts18:21
*** david-lyle has quit IRC18:35
*** harlowja has quit IRC18:44
*** david-lyle has joined #openstack-meeting-cp19:24
*** sdake has quit IRC19:35
*** sdake has joined #openstack-meeting-cp19:38
*** openstack has joined #openstack-meeting-cp19:40
*** ChanServ sets mode: +o openstack19:40
*** sdake has quit IRC19:43
*** david-lyle has quit IRC19:57
*** amrith is now known as _amrith_20:07
*** harlowja has joined #openstack-meeting-cp20:24
*** piet_ has joined #openstack-meeting-cp20:34
*** sheel has quit IRC20:35
*** _amrith_ is now known as amrith21:30
*** markvoelker has quit IRC21:58
*** xyang1 has quit IRC21:59
*** markvoelker has joined #openstack-meeting-cp22:58
*** markvoelker has quit IRC23:03
*** david-lyle has joined #openstack-meeting-cp23:20
*** sdake has joined #openstack-meeting-cp23:54
*** sdake_ has quit IRC23:56
*** sdake has quit IRC23:57
*** sdake has joined #openstack-meeting-cp23:57
*** sdague has quit IRC23:58

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!