16:01:24 <smcginnis> #startmeeting Cinder
16:01:25 <openstack> Meeting started Wed May 18 16:01:24 2016 UTC and is due to finish in 60 minutes.  The chair is smcginnis. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:01:26 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:01:27 <e0ne> hi
16:01:28 <openstack> The meeting name has been set to 'cinder'
16:01:28 <diablo_rojo1> hello
16:01:29 <eharney> hi
16:01:29 * jungleboyj takes a break from packing my office.
16:01:30 <jgregor> Hello!
16:01:30 <kmartin> hi
16:01:31 <Swanson> Hello
16:01:36 <smcginnis> Courtesy ping dulek duncant eharney geguileo winston-d e0ne jungleboyj jgriffith thingee smcginnis hemna xyang tbarron scottda erlon rhedlind jbernard _alastor_ vincent_hou kmartin patrickeast sheel dongwenjuan JaniceLee cFouts Thelo vivekd adrianofr mtanino yuriy_n17 karlamrhein diablo_rojo jay.xu jgregor baumann rajinir
16:01:38 <geguileo> Hi!
16:01:42 <tbarron> hi
16:01:45 <adrianofr> hi
16:01:47 <mtanino> hi
16:01:50 <smcginnis> #link https://wiki.openstack.org/wiki/CinderMeetings#Next_meeting Agenda
16:01:52 <baumann> Hello!
16:01:53 <hemna> mep
16:01:53 <dulek> o/
16:01:54 <Swanson> jungleboyj, they finally found out about the kickbacks?
16:01:54 <flip214> hi
16:01:54 <kfarr> hello!
16:01:59 <scottda> hey
16:02:03 <jungleboyj> Swanson: ;-)
16:02:05 <xingyang> hi
16:02:06 <karthikp> Hi
16:02:10 <fernnest> hi
16:02:17 <gnarld_> hi
16:02:21 <Yogi1> hi
16:02:22 <ntpttr_> o/
16:02:27 <smcginnis> #topic Announcements
16:02:42 <smcginnis> Just want to point out we are two weeks out from the N-1 milestone.
16:02:46 <patrickeast> hi
16:02:57 <smcginnis> No extra deadlines for Cinder for that, but it is a good checkpoint.
16:03:13 <smcginnis> Definitely any new drivers should be submitting code by then, hopefully.
16:03:27 <smcginnis> Also, good progress on reviewing specs.
16:03:43 <smcginnis> Those seem to be moving along a little smoother than last cycle at this point.
16:04:10 <smcginnis> It would be great if we can stick to our spec freeze at N-2, but I know that's not always possible.
16:04:33 <thingee> o/
16:04:41 <smcginnis> #topic Check 'thin_provisioning_support' in extra specs
16:04:50 <smcginnis> No name, but I believe that was you xyang1 ?
16:04:55 <xingyang> hi
16:04:57 <xingyang> https://review.openstack.org/#/c/315352/
16:05:13 <xingyang> Some backends can do thin and thick in the same pool. The existing scheduler logic is pro thin.  It does not work well for thick.
16:05:25 <xingyang> If a driver reports thin_provisioning_support=True, it uses the thin logic. This could potentially lead to over provisioning if the user is creating a thick volume.
16:05:35 <xingyang> So the patch proposes to check extra specs to decide whether to use thin or thick logic.
16:06:05 <hemna> our driver can do both thin and thick in the same pool
16:06:15 <smcginnis> Since thin would be the default, would it be better to have the spec call out thick?
16:06:21 <smcginnis> hemna: Ours as well.
16:06:27 <xingyang> hemna: ours too
16:06:31 <jgriffith> Can we not make this so hard
16:06:39 <smcginnis> Well, technically everything is thin, but we can preallocate to make it "thick".
16:06:44 <eharney> is this patch ensuring that the driver actually does this, or just that the scheduler is using this for calculation of free space?
16:06:44 <xingyang> jgriffith: suggestion?
16:06:55 <jgriffith> Just do one or the other
16:06:57 <Swanson> smcginnis, technically we just do thin.
16:07:16 <jgriffith> or make up some fancy calculations in your driver
16:07:19 <xingyang> jgriffith: currently we allow both, but the scheduler does not work right
16:07:29 <jgriffith> xingyang: Yeah, I get it
16:07:38 <jgriffith> I'm saying just have thin-pools and thick-pools
16:07:54 <smcginnis> jgriffith: Only problem I have with that is I have one pool.
16:07:55 <hemna> so....fail on create in the driver ?
16:08:02 * hemna sadness
16:08:05 <jgriffith> just because a backend can do something doesn't mean Cinder shojld care about it
16:08:15 <eharney> or that we should provide it to a user for them to care about it
16:08:18 <jgriffith> smcginnis: so the admin picks :)
16:08:21 <jgriffith> one or the other
16:08:34 <DuncanT> jgriffith: Not having to have pools and letting the space be shared across two types is rather nicer, I'd have thought
16:08:34 <xingyang> jgriffith: we currently allow that
16:08:46 <xingyang> jgriffith: now we suddently make it a restriction?
16:08:55 <smcginnis> Since I am only thin provisioned, the only way I can provide thick provisioning is for it to be explicitly requested.
16:09:04 <jgriffith> xingyang: yes, that's my point
16:09:08 <DuncanT> jgriffith: Not sure if it is nice enough to make the code complex though
16:09:14 <hemna> :(
16:09:24 <eharney> the code in this area already needs some work
16:09:31 <smcginnis> But I suppose we can just say we can't provide thick. Or provide out own extra spec.
16:09:55 <jgriffith> smcginnis: +1 for extra-spec
16:10:01 <jgriffith> just create thin and thick types
16:10:06 <xingyang> jgriffith: but that will be removing the functionality?
16:10:22 <hemna> ours is driven by extra specs
16:10:25 <hemna> thin vs. thick
16:10:30 <hemna> but that can still happen on the same pool.
16:10:33 <patrickeast> im confused, isn't all this already driven by specs?
16:10:36 <jgriffith> hemna: horray!  That's how it should be IMO
16:10:38 <hemna> patrickeast, +1
16:10:48 * hemna is confused
16:10:57 <patrickeast> the scheduler code is just so that it can decide when a backend has free space or not
16:11:00 <xingyang> hemna: I see that your driver reports thin_provisioning_support and thick_provisioning_support to True
16:11:06 <patrickeast> nothing to do with how the volume actually got created
16:11:30 <xingyang> patrickeast: there is flaw in the scheduler logic
16:11:31 <eharney> patrickeast: i was confused about that too
16:11:40 <smcginnis> patrickeast: So isn't that the key point here? This patch tries to make the scheduler aware of that and do the right evaluation.
16:11:48 <smcginnis> Which it can't really do otherwise.
16:12:14 <patrickeast> yea, which seems like a nice de-duplication instead of all the drivers calculating virtual free space or something in their update stats call, right?
16:12:14 <smcginnis> So if we report thin and allow over provisioning, and they send down an exta spec requesting thick, it kind of messes some things up.
16:12:28 <patrickeast> i guess i'm not sure why this is controversial
16:12:42 <hemna> patrickeast, +1
16:13:29 <smcginnis> eharney: I think you had the original objection here.
16:13:29 <xingyang> How about I write a spec and hopefully explain things better?
16:13:46 <tbarron> xingyang: +1
16:13:50 <eharney> my main objection is that this was written as a bug but it's not clear whether it's actually a bug... or what the use case is etc
16:14:01 <geguileo> patrickeast: But drivers will still need to calculate it and send it back
16:14:01 <eharney> this code is already overly complex and we don't need to make it worse without being totally clear on what's going on
16:14:02 <smcginnis> xingyang: Seems a little overkill, but that may help with the communication and understanding here.
16:14:16 <Swanson> xingyang, +1
16:14:19 <smcginnis> eharney: Fair point.
16:14:26 <diablo_rojo> xingyang: I think that sounds good. Just to get us all on the same page
16:14:27 <xingyang> smcginnis: seems that we are not on the same page, so a spec may help, or it could be worse:)
16:14:37 <smcginnis> True
16:14:38 <diablo_rojo> xingyang: +1
16:14:48 <tbarron> this could be an opportunity to refactor that code and make it clearer
16:14:54 <eharney> a bug about "the user wanting the volume to be thick or thin" isn't a bug to me, because that isn't something for the user to care about IMO
16:14:54 <geguileo> tbarron: +1
16:15:03 <e0ne> tbarron>: good point
16:15:16 <eharney> so yes, i'm all for speccing out how this is all supposed to work and seeing how we can improve it
16:15:18 <smcginnis> tbarron: That would be a good outcome if nothing else from this.
16:15:32 <diablo_rojo> eharney: Yeah I don't think the user should know about that really, right?
16:15:37 <eharney> diablo_rojo: right
16:15:38 <xingyang> tbarron: I was trying to avoid going through the same spec review that I had to go thru in Kilo:)
16:15:44 <smcginnis> #action xingyang to write spec for thin provisioning details
16:15:45 <tbarron> there are a lot of double negatives in that code now :)
16:15:51 <xingyang> tbarron: but you wouldn’t let me escape:(
16:16:03 <smcginnis> It's a trap!
16:16:05 <tbarron> xingyang: :)
16:16:11 <diablo_rojo> tbarron: Two wrongs make a right, right? :)
16:16:15 <jungleboyj> xingyang: Run!
16:16:21 <xingyang> :)
16:16:23 <tbarron> wrong, wrong!
16:16:25 <smcginnis> diablo_rojo: That's not incorrect. :)
16:16:38 <diablo_rojo> smcginnis: :)
16:16:43 <smcginnis> xingyang: Good enough?
16:16:49 <eharney> sounds good to me
16:16:50 <xingyang> smcginnis: yes
16:16:54 <smcginnis> Thanks!
16:16:56 * jungleboyj loves double negatives.
16:17:01 <smcginnis> #topic Integrate Castellan for key management
16:17:11 <smcginnis> Another one with no name on it.
16:17:13 <kfarr> That one was mine!
16:17:23 <smcginnis> kfarr: Great, all yours!
16:17:27 <kfarr> To give a little history, when the Cinder volume encryption feature was implemented, identical key manager interface code was introduced into both Nova add Cinder
16:17:35 <kfarr> The key manager interface code was copied over into a new library Castellan, with the intent that Castellan would replace the key manager code in Cinder and Nova and be used for key manager needs in other projects.
16:17:48 <kfarr> The code to integrate Castellan into Nova has already merged: https://review.openstack.org/#/c/309614/
16:17:56 <kfarr> But there's been a lot more discussion on the Cinder spec: https://review.openstack.org/#/c/309614/
16:18:03 <kfarr> Cinder code here: https://review.openstack.org/#/c/280492/ (dependent on this bug fix: https://review.openstack.org/#/c/251503/)
16:18:18 <kfarr> So to help things along, I wanted to have this discussion real-time over chat, to answer any questions or comments
16:18:19 <smcginnis> Same link for the cinder spec and the nova one htere.
16:18:21 <smcginnis> *there
16:18:33 <kfarr> one sec
16:18:44 <kfarr> cinder spec: https://review.openstack.org/#/c/247577/
16:19:00 <smcginnis> #link https://review.openstack.org/#/c/280492/ Cinder castellan patch
16:19:13 <smcginnis> #link https://review.openstack.org/#/c/247577/ Cinder castellan spec
16:19:16 <smcginnis> kfarr: Thanks
16:19:28 <kfarr> I believe DuncanT had a -1 on it?
16:19:36 <DuncanT> So my comment here is the same as it was initially: I want the conff key manager to be behind castellan, so we can remove it entirely from the cinder code and so that more of the castalan code path gets exercised in the gate
16:20:11 * redrobot pokes head in
16:20:26 <hemna> kfarr, what about this patch in os-brick to add the encryptors support ?  https://review.openstack.org/#/c/247372/
16:20:51 <kfarr> DuncanT the way that it's been refactored though the ConfKeyManager still uses Castellan
16:21:01 <smcginnis> DuncanT: So no in depth thought on my part, but patrickeast's comment on there makes some sense to me.
16:21:15 <lixiaoy11> hemna: integrate Castellan is to move code duplication about keymanager
16:21:22 <smcginnis> DuncanT: Do you think that's acceptable?
16:22:01 <hemna> ok, just they key management then.
16:22:44 <smcginnis> So is the os-brick patch still needed?
16:22:45 <DuncanT> smcginnis: Given the initial answer was 'we don't like the conf key manager, it isn't going into castallan since it isn't secure', it seemed like the second step would never happen
16:22:52 * smcginnis needs to read up on these again
16:23:16 <kfarr> smcginnis hemna, I believe the os-brick patch is still needed
16:23:21 <lixiaoy11> smcginnis:  yes, os-brick patch is a step to attach encrypted volume. it is needed
16:23:25 <smcginnis> OK, thanks.
16:23:41 <smcginnis> DuncanT: OK, I remember some of the comments there.
16:23:44 <hemna> kfarr, ok coolio.
16:23:56 <smcginnis> DuncanT: Should that hold up the whole effort though?
16:24:00 <smcginnis> Not sure, just asking.
16:24:02 <DuncanT> It started to read as 'castalan is a library to abstract away key management. Except that key manager, which we don't like, so you're still going to have to have a second abstraction layer to get that one', which is daft
16:24:18 <DuncanT> I think it should hold it up, yes, since it is pretty fundamental
16:24:45 <kfarr> DuncanT did you get a chance to look at the code, not just the spec?
16:25:03 <DuncanT> kfarr: I've not looked at it for a while
16:26:18 <lixiaoy11> smcginnis: DuncanT: as it is reasonable to not put ConfKeyManager in Castellan. If we put it into some other olso library, seems it needs time to do it.
16:26:57 <smcginnis> Where would it go? If it's used by both, I agree it should not be duplicated.
16:27:20 <kfarr> The ConfKeyManager is only used by Nova and Cinder in DevStack
16:27:44 <DuncanT> lixiaoy11: I don't want two layers of key manager abstraction in cinder, it's daft - I don't care if we keep it in-tree as a castallan plugin if that is doable, I just don't want using confkey to bypass castalan
16:28:03 <kfarr> DuncanT I wouldn't call it a second abstraction layer
16:28:59 <DuncanT> kfarr: I do, looking at the code
16:29:00 <kfarr> DuncanT With the refactoring, the ConfKeyManager becomes an implementation of the Castellan key manager interface
16:29:14 <DuncanT> kfarr: I'll look at the code again
16:29:51 <DuncanT> kfarr: and get back to you ASAP
16:30:22 <kfarr> Ok, did anyone else have comments about it?  I don't want to take up too much of the meeting time
16:32:06 <kfarr> smcginnis, since DuncanT will follow up on the spec later, I suppose it's time for the next agenda item?
16:32:36 <smcginnis> kfarr: OK, thanks for bringing it up here. Looks like it definitely could use some more attention.
16:32:41 <DuncanT> kfarr: At a glance it might be that the spec and the code don't quite agree, with the code being good. If so, it should be very easy to ammend the spec to be totally clear
16:33:00 <DuncanT> kfarr: I'll spend some time on it after the meeting
16:33:13 <smcginnis> DuncanT: Thanks!
16:33:28 <smcginnis> #topic Release new python-cinderclient
16:33:29 <kfarr> Thanks DuncanT!
16:33:43 <smcginnis> No one wants to put there names on things this week I guess. :)
16:33:50 <DuncanT> This was a topic I added
16:33:51 <smcginnis> But yeah, we probably need to.
16:33:57 <smcginnis> DuncanT: :)
16:34:07 <smcginnis> Anyone aware of anything outstanding we should try to get in?
16:34:15 <smcginnis> I can get a release requested this afternoon otherwise.
16:34:49 <DuncanT> But I've nothing to say that isn't in the agenda - I don't know of anything outstanding for mataka servers, and I want a release without too much newton support in it, ideally
16:35:24 <smcginnis> Yeah, I think I had planned on getting one sooner, then waited for something, then by the time that made it got distracted. :/
16:35:33 <smcginnis> So I really should get that going.
16:35:41 <smcginnis> I did take a look at the outstanding reviews.
16:35:43 <DuncanT> :-)
16:35:59 <smcginnis> Nothing looked critical to me, so if anyone is aware of anything please ping me on it soon.
16:36:17 <smcginnis> But it did bring to my attention - we could really use some more reviews on client patches.
16:36:34 <smcginnis> There are a lot sitting out there that could use some eyes.
16:36:43 <aimeeu> I'll take a look fwiw
16:36:48 <smcginnis> aimeeu: Thanks!
16:37:05 <smcginnis> #action smcginnis to request new python-cinderclient release
16:37:10 <diablo_rojo> smcginnis: I can review some tongiht
16:37:13 <diablo_rojo> *tonight
16:37:19 <smcginnis> diablo_rojo: Cool, thanks!
16:37:29 <smcginnis> #topic Open Discussion
16:37:40 <e0ne> i've got one question
16:37:42 <smcginnis> The floor is open. Anyone got some bikeshedding? :)
16:37:56 <e0ne> eharney proposed https://review.openstack.org/#/c/316993/
16:38:24 <e0ne> personally, I like this idea, but there was a lot of tbarron's patches with fake IDs
16:38:28 <karthikp> Hi, I could use some help suggestion and review
16:38:39 <e0ne> what solution will we prefer?
16:38:42 <karthikp> https://review.openstack.org/#/c/309605/
16:38:48 <karthikp> https://review.openstack.org/#/c/313179/
16:38:54 <karthikp> https://review.openstack.org/#/c/300708/
16:39:26 <smcginnis> Seems in general where it doesn't really matter it's good to use the fakes. Where we want to be able to untangle what happened, we want unique IDs.
16:39:37 <smcginnis> tbarron: Any thoughts on that Mr. UUID? :)
16:39:48 <jgriffith> smcginnis: but you can use the fakes as unique no?
16:39:51 <tbarron> the fake IDs were just a step in the right direction, not the final story
16:40:03 <jgriffith> smcginnis: there's a pretty long list of fakes that are unique
16:40:04 <smcginnis> jgriffith: I thought so...
16:40:04 <e0ne> smcginnis: both approaches are good, but  should we use them together?
16:40:05 <tbarron> it was a way to get us past the invalid uuids
16:40:13 <jgriffith> I'm not saying I have a preference one way or the other
16:40:23 <jgriffith> just saying that either way *should* work
16:40:26 <e0ne> jgriffith: but eharney proposed to use unique IDs
16:40:33 <smcginnis> tbarron, eharney: So can we switch these over to use unique fakes?
16:40:45 <tbarron> we had no real hope of getting that much code done and putting in a ratchet by doing all of them unique I thought at least
16:40:52 <jgriffith> e0ne: yes, I'm saying you could do the same thing:  fake_uuids.Fake1, .... Fake3
16:40:54 <jgriffith> get it?
16:41:00 <tbarron> but I have no problem with making progress on that front now
16:41:12 <e0ne> jgriffith: oh.. good idea:)
16:41:23 <smcginnis> There are multiple UUIDs in there, so each could use its own unique one.
16:41:29 <DuncanT> Unique and distinctive if not actually fake (deadbeef etc) sounds good to me
16:41:32 <smcginnis> Basically what jgriffith is saying. :)
16:41:35 <jgriffith> again, I'm fine with either approach.  I just don't know what's best in terms of organizing and keeping things clear going forward
16:41:44 <smcginnis> Same
16:41:48 * dulek thinks smcginnis made a mistake by asking for bikeshedding.
16:41:53 <smcginnis> :)
16:42:01 <tbarron> dulek: +1 :)
16:42:05 <smcginnis> Gotta get it out at some point. :)
16:42:14 * DuncanT is building an actual bikeshed at the weekend, but we've already bought the paint
16:42:20 <dulek> Oh, better here than in specs. ;)
16:42:21 <jgriffith> DuncanT: https://github.com/openstack/cinder/blob/master/cinder/tests/unit/fake_constants.py
16:42:25 <smcginnis> DuncanT: What color? :D
16:42:30 <tbarron> DuncanT: it's not too late!
16:42:40 <DuncanT> Army green - it was cheap
16:42:43 <jgriffith> So you can make a list of "fakebackup-1 ..... n" and just ref them
16:42:50 <tbarron> DuncanT: once you paint it you can't change the color for two cycles
16:42:55 <smcginnis> DuncanT: Hah, popular color over there I suppose. :)
16:43:01 <smcginnis> tbarron: LOL
16:43:10 <jgriffith> some day I'd like to seed a prng and just generate repeatable ID's rather than have a list... but whatever
16:43:10 <diablo_rojo> smcginnis: LOL
16:43:16 <DuncanT> jgriffith: Got it. Thanks.
16:44:02 <smcginnis> jgriffith: Would be nice to have repeatable generated IDs rather than maintaining the long list.
16:44:05 <jgriffith> or hell... just make FAKE_UUIDS[xxxxxx......zzzzzz]  and in the tests use "FAKE-UUID[0]" [1] or whatever
16:44:05 <smcginnis> But meh
16:44:15 <DuncanT> jgriffith: I'd personally have put an eyeballable prefix on them (c0ffee, deadbeef, feed, whatever) but it's a small thing
16:44:46 <jgriffith> DuncanT: prefix on the UUID?  That defeats the whole point no?
16:44:57 <jgriffith> whatever.. I shouldn't dreail convo here ;)
16:45:11 <jgriffith> derail even :)
16:45:12 <DuncanT> jgriffith: The first few digits of the uuid, not an actual prefix
16:45:58 <cFouts> DuncanT +1
16:46:57 <DuncanT> jgriffith: If all fake UUIDs start badc-0ffe-eXXX-XXXX-XXXX then it's easy to spot when they're leaking
16:47:19 <jungleboyj> DuncanT: That makes sense.
16:47:20 <smcginnis> Bad coffee? That's terrible.
16:47:26 <smcginnis> :)
16:47:31 <DuncanT> or badc-0deX-XXXX-XXXX-XXXX if you want to be pointed
16:47:34 <jungleboyj> smcginnis: Certainly don't want that leaking.
16:47:42 <smcginnis> ha
16:48:25 <tbarron> so, has anyone been running cinder with zookeeper under tooz lately?
16:48:37 <tbarron> nice weather here
16:49:23 <smcginnis> tbarron: Have you tried it?
16:49:34 <smcginnis> tooz that is.
16:49:48 <tbarron> smcginnis: not with zk underneath
16:49:50 <e0ne> DuncanT: +1, I like this proposal
16:50:04 <dulek> tbarron: I don't think we've converted any locks to actually use Tooz.
16:50:07 <smcginnis> tbarron: Me neither.
16:50:26 <dulek> tbarron: That's probably badly advertised in the release notes…
16:50:39 <DuncanT> tbarron: If you're interested in testing, I can give you a fault injection patch than will give you a good chance of finding breakage
16:51:06 <smcginnis> Shameless plug: I'd love feedback on https://review.openstack.org/#/c/297140/ to know if I'm completely off the rails.
16:51:10 <tbarron> dulek: yeah, i'm just getting a sense of where we are (and personally am avoiding uuids at any cost)
16:51:17 <smcginnis> If that goes through, there are some follow up patches I would like to do.
16:51:37 <smcginnis> Like using the output of "tox -e gendriverlist" to make this sane: http://docs.openstack.org/developer/cinder/drivers.html
16:51:49 <dulek> tbarron: bluex is the person to ask on status. Not sure when he'll be online.
16:51:50 <smcginnis> Not even sure what the page is supposed to be conveying.
16:51:54 <diablo_rojo> smcginnis: I think interface checks would be good
16:52:06 <tbarron> dulek: ty!
16:52:17 <diablo_rojo> smcginnis: Your demo at the summit showed how useful they can be.
16:52:25 <smcginnis> Docs too. We can point new driver developers to the sphynx output of cinder.interface classes.
16:52:27 * dulek wanted to look into Tooz stuff this week, but still gets distracted.
16:52:36 <smcginnis> diablo_rojo: Cool, glad it at least made some sense. :D
16:53:00 <diablo_rojo> smcginnis: Seemed like a pretty easy way to get tons of info about all the drivers
16:53:10 <DuncanT> smcginnis: I've been meaning to get to that patch, will try harder
16:53:24 <DuncanT> smcginnis: IT certainly seems to achieve good things
16:53:51 <smcginnis> I'm thinking so, but I certainly have a biased view. ;)
16:55:11 <smcginnis> Anything else?
16:55:35 <smcginnis> K, thanks everyone.
16:55:39 <jungleboyj> Thanks.
16:55:45 <smcginnis> #endmeeting