16:01:24 #startmeeting Cinder 16:01:25 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 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 16:01:27 hi 16:01:28 The meeting name has been set to 'cinder' 16:01:28 hello 16:01:29 hi 16:01:29 * jungleboyj takes a break from packing my office. 16:01:30 Hello! 16:01:30 hi 16:01:31 Hello 16:01:36 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 Hi! 16:01:42 hi 16:01:45 hi 16:01:47 hi 16:01:50 #link https://wiki.openstack.org/wiki/CinderMeetings#Next_meeting Agenda 16:01:52 Hello! 16:01:53 mep 16:01:53 o/ 16:01:54 jungleboyj, they finally found out about the kickbacks? 16:01:54 hi 16:01:54 hello! 16:01:59 hey 16:02:03 Swanson: ;-) 16:02:05 hi 16:02:06 Hi 16:02:10 hi 16:02:17 hi 16:02:21 hi 16:02:22 o/ 16:02:27 #topic Announcements 16:02:42 Just want to point out we are two weeks out from the N-1 milestone. 16:02:46 hi 16:02:57 No extra deadlines for Cinder for that, but it is a good checkpoint. 16:03:13 Definitely any new drivers should be submitting code by then, hopefully. 16:03:27 Also, good progress on reviewing specs. 16:03:43 Those seem to be moving along a little smoother than last cycle at this point. 16:04:10 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 o/ 16:04:41 #topic Check 'thin_provisioning_support' in extra specs 16:04:50 No name, but I believe that was you xyang1 ? 16:04:55 hi 16:04:57 https://review.openstack.org/#/c/315352/ 16:05:13 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 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 So the patch proposes to check extra specs to decide whether to use thin or thick logic. 16:06:05 our driver can do both thin and thick in the same pool 16:06:15 Since thin would be the default, would it be better to have the spec call out thick? 16:06:21 hemna: Ours as well. 16:06:27 hemna: ours too 16:06:31 Can we not make this so hard 16:06:39 Well, technically everything is thin, but we can preallocate to make it "thick". 16:06:44 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 jgriffith: suggestion? 16:06:55 Just do one or the other 16:06:57 smcginnis, technically we just do thin. 16:07:16 or make up some fancy calculations in your driver 16:07:19 jgriffith: currently we allow both, but the scheduler does not work right 16:07:29 xingyang: Yeah, I get it 16:07:38 I'm saying just have thin-pools and thick-pools 16:07:54 jgriffith: Only problem I have with that is I have one pool. 16:07:55 so....fail on create in the driver ? 16:08:02 * hemna sadness 16:08:05 just because a backend can do something doesn't mean Cinder shojld care about it 16:08:15 or that we should provide it to a user for them to care about it 16:08:18 smcginnis: so the admin picks :) 16:08:21 one or the other 16:08:34 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 jgriffith: we currently allow that 16:08:46 jgriffith: now we suddently make it a restriction? 16:08:55 Since I am only thin provisioned, the only way I can provide thick provisioning is for it to be explicitly requested. 16:09:04 xingyang: yes, that's my point 16:09:08 jgriffith: Not sure if it is nice enough to make the code complex though 16:09:14 :( 16:09:24 the code in this area already needs some work 16:09:31 But I suppose we can just say we can't provide thick. Or provide out own extra spec. 16:09:55 smcginnis: +1 for extra-spec 16:10:01 just create thin and thick types 16:10:06 jgriffith: but that will be removing the functionality? 16:10:22 ours is driven by extra specs 16:10:25 thin vs. thick 16:10:30 but that can still happen on the same pool. 16:10:33 im confused, isn't all this already driven by specs? 16:10:36 hemna: horray! That's how it should be IMO 16:10:38 patrickeast, +1 16:10:48 * hemna is confused 16:10:57 the scheduler code is just so that it can decide when a backend has free space or not 16:11:00 hemna: I see that your driver reports thin_provisioning_support and thick_provisioning_support to True 16:11:06 nothing to do with how the volume actually got created 16:11:30 patrickeast: there is flaw in the scheduler logic 16:11:31 patrickeast: i was confused about that too 16:11:40 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 Which it can't really do otherwise. 16:12:14 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 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 i guess i'm not sure why this is controversial 16:12:42 patrickeast, +1 16:13:29 eharney: I think you had the original objection here. 16:13:29 How about I write a spec and hopefully explain things better? 16:13:46 xingyang: +1 16:13:50 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 patrickeast: But drivers will still need to calculate it and send it back 16:14:01 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 xingyang: Seems a little overkill, but that may help with the communication and understanding here. 16:14:16 xingyang, +1 16:14:19 eharney: Fair point. 16:14:26 xingyang: I think that sounds good. Just to get us all on the same page 16:14:27 smcginnis: seems that we are not on the same page, so a spec may help, or it could be worse:) 16:14:37 True 16:14:38 xingyang: +1 16:14:48 this could be an opportunity to refactor that code and make it clearer 16:14:54 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 tbarron: +1 16:15:03 tbarron>: good point 16:15:16 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 tbarron: That would be a good outcome if nothing else from this. 16:15:32 eharney: Yeah I don't think the user should know about that really, right? 16:15:37 diablo_rojo: right 16:15:38 tbarron: I was trying to avoid going through the same spec review that I had to go thru in Kilo:) 16:15:44 #action xingyang to write spec for thin provisioning details 16:15:45 there are a lot of double negatives in that code now :) 16:15:51 tbarron: but you wouldn’t let me escape:( 16:16:03 It's a trap! 16:16:05 xingyang: :) 16:16:11 tbarron: Two wrongs make a right, right? :) 16:16:15 xingyang: Run! 16:16:21 :) 16:16:23 wrong, wrong! 16:16:25 diablo_rojo: That's not incorrect. :) 16:16:38 smcginnis: :) 16:16:43 xingyang: Good enough? 16:16:49 sounds good to me 16:16:50 smcginnis: yes 16:16:54 Thanks! 16:16:56 * jungleboyj loves double negatives. 16:17:01 #topic Integrate Castellan for key management 16:17:11 Another one with no name on it. 16:17:13 That one was mine! 16:17:23 kfarr: Great, all yours! 16:17:27 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 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 The code to integrate Castellan into Nova has already merged: https://review.openstack.org/#/c/309614/ 16:17:56 But there's been a lot more discussion on the Cinder spec: https://review.openstack.org/#/c/309614/ 16:18:03 Cinder code here: https://review.openstack.org/#/c/280492/ (dependent on this bug fix: https://review.openstack.org/#/c/251503/) 16:18:18 So to help things along, I wanted to have this discussion real-time over chat, to answer any questions or comments 16:18:19 Same link for the cinder spec and the nova one htere. 16:18:21 *there 16:18:33 one sec 16:18:44 cinder spec: https://review.openstack.org/#/c/247577/ 16:19:00 #link https://review.openstack.org/#/c/280492/ Cinder castellan patch 16:19:13 #link https://review.openstack.org/#/c/247577/ Cinder castellan spec 16:19:16 kfarr: Thanks 16:19:28 I believe DuncanT had a -1 on it? 16:19:36 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 kfarr, what about this patch in os-brick to add the encryptors support ? https://review.openstack.org/#/c/247372/ 16:20:51 DuncanT the way that it's been refactored though the ConfKeyManager still uses Castellan 16:21:01 DuncanT: So no in depth thought on my part, but patrickeast's comment on there makes some sense to me. 16:21:15 hemna: integrate Castellan is to move code duplication about keymanager 16:21:22 DuncanT: Do you think that's acceptable? 16:22:01 ok, just they key management then. 16:22:44 So is the os-brick patch still needed? 16:22:45 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 smcginnis hemna, I believe the os-brick patch is still needed 16:23:21 smcginnis: yes, os-brick patch is a step to attach encrypted volume. it is needed 16:23:25 OK, thanks. 16:23:41 DuncanT: OK, I remember some of the comments there. 16:23:44 kfarr, ok coolio. 16:23:56 DuncanT: Should that hold up the whole effort though? 16:24:00 Not sure, just asking. 16:24:02 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 I think it should hold it up, yes, since it is pretty fundamental 16:24:45 DuncanT did you get a chance to look at the code, not just the spec? 16:25:03 kfarr: I've not looked at it for a while 16:26:18 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 Where would it go? If it's used by both, I agree it should not be duplicated. 16:27:20 The ConfKeyManager is only used by Nova and Cinder in DevStack 16:27:44 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 DuncanT I wouldn't call it a second abstraction layer 16:28:59 kfarr: I do, looking at the code 16:29:00 DuncanT With the refactoring, the ConfKeyManager becomes an implementation of the Castellan key manager interface 16:29:14 kfarr: I'll look at the code again 16:29:51 kfarr: and get back to you ASAP 16:30:22 Ok, did anyone else have comments about it? I don't want to take up too much of the meeting time 16:32:06 smcginnis, since DuncanT will follow up on the spec later, I suppose it's time for the next agenda item? 16:32:36 kfarr: OK, thanks for bringing it up here. Looks like it definitely could use some more attention. 16:32:41 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 kfarr: I'll spend some time on it after the meeting 16:33:13 DuncanT: Thanks! 16:33:28 #topic Release new python-cinderclient 16:33:29 Thanks DuncanT! 16:33:43 No one wants to put there names on things this week I guess. :) 16:33:50 This was a topic I added 16:33:51 But yeah, we probably need to. 16:33:57 DuncanT: :) 16:34:07 Anyone aware of anything outstanding we should try to get in? 16:34:15 I can get a release requested this afternoon otherwise. 16:34:49 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 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 So I really should get that going. 16:35:41 I did take a look at the outstanding reviews. 16:35:43 :-) 16:35:59 Nothing looked critical to me, so if anyone is aware of anything please ping me on it soon. 16:36:17 But it did bring to my attention - we could really use some more reviews on client patches. 16:36:34 There are a lot sitting out there that could use some eyes. 16:36:43 I'll take a look fwiw 16:36:48 aimeeu: Thanks! 16:37:05 #action smcginnis to request new python-cinderclient release 16:37:10 smcginnis: I can review some tongiht 16:37:13 *tonight 16:37:19 diablo_rojo: Cool, thanks! 16:37:29 #topic Open Discussion 16:37:40 i've got one question 16:37:42 The floor is open. Anyone got some bikeshedding? :) 16:37:56 eharney proposed https://review.openstack.org/#/c/316993/ 16:38:24 personally, I like this idea, but there was a lot of tbarron's patches with fake IDs 16:38:28 Hi, I could use some help suggestion and review 16:38:39 what solution will we prefer? 16:38:42 https://review.openstack.org/#/c/309605/ 16:38:48 https://review.openstack.org/#/c/313179/ 16:38:54 https://review.openstack.org/#/c/300708/ 16:39:26 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 tbarron: Any thoughts on that Mr. UUID? :) 16:39:48 smcginnis: but you can use the fakes as unique no? 16:39:51 the fake IDs were just a step in the right direction, not the final story 16:40:03 smcginnis: there's a pretty long list of fakes that are unique 16:40:04 jgriffith: I thought so... 16:40:04 smcginnis: both approaches are good, but should we use them together? 16:40:05 it was a way to get us past the invalid uuids 16:40:13 I'm not saying I have a preference one way or the other 16:40:23 just saying that either way *should* work 16:40:26 jgriffith: but eharney proposed to use unique IDs 16:40:33 tbarron, eharney: So can we switch these over to use unique fakes? 16:40:45 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 e0ne: yes, I'm saying you could do the same thing: fake_uuids.Fake1, .... Fake3 16:40:54 get it? 16:41:00 but I have no problem with making progress on that front now 16:41:12 jgriffith: oh.. good idea:) 16:41:23 There are multiple UUIDs in there, so each could use its own unique one. 16:41:29 Unique and distinctive if not actually fake (deadbeef etc) sounds good to me 16:41:32 Basically what jgriffith is saying. :) 16:41:35 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 Same 16:41:48 * dulek thinks smcginnis made a mistake by asking for bikeshedding. 16:41:53 :) 16:42:01 dulek: +1 :) 16:42:05 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 Oh, better here than in specs. ;) 16:42:21 DuncanT: https://github.com/openstack/cinder/blob/master/cinder/tests/unit/fake_constants.py 16:42:25 DuncanT: What color? :D 16:42:30 DuncanT: it's not too late! 16:42:40 Army green - it was cheap 16:42:43 So you can make a list of "fakebackup-1 ..... n" and just ref them 16:42:50 DuncanT: once you paint it you can't change the color for two cycles 16:42:55 DuncanT: Hah, popular color over there I suppose. :) 16:43:01 tbarron: LOL 16:43:10 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 smcginnis: LOL 16:43:16 jgriffith: Got it. Thanks. 16:44:02 jgriffith: Would be nice to have repeatable generated IDs rather than maintaining the long list. 16:44:05 or hell... just make FAKE_UUIDS[xxxxxx......zzzzzz] and in the tests use "FAKE-UUID[0]" [1] or whatever 16:44:05 But meh 16:44:15 jgriffith: I'd personally have put an eyeballable prefix on them (c0ffee, deadbeef, feed, whatever) but it's a small thing 16:44:46 DuncanT: prefix on the UUID? That defeats the whole point no? 16:44:57 whatever.. I shouldn't dreail convo here ;) 16:45:11 derail even :) 16:45:12 jgriffith: The first few digits of the uuid, not an actual prefix 16:45:58 DuncanT +1 16:46:57 jgriffith: If all fake UUIDs start badc-0ffe-eXXX-XXXX-XXXX then it's easy to spot when they're leaking 16:47:19 DuncanT: That makes sense. 16:47:20 Bad coffee? That's terrible. 16:47:26 :) 16:47:31 or badc-0deX-XXXX-XXXX-XXXX if you want to be pointed 16:47:34 smcginnis: Certainly don't want that leaking. 16:47:42 ha 16:48:25 so, has anyone been running cinder with zookeeper under tooz lately? 16:48:37 nice weather here 16:49:23 tbarron: Have you tried it? 16:49:34 tooz that is. 16:49:48 smcginnis: not with zk underneath 16:49:50 DuncanT: +1, I like this proposal 16:50:04 tbarron: I don't think we've converted any locks to actually use Tooz. 16:50:07 tbarron: Me neither. 16:50:26 tbarron: That's probably badly advertised in the release notes… 16:50:39 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 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 dulek: yeah, i'm just getting a sense of where we are (and personally am avoiding uuids at any cost) 16:51:17 If that goes through, there are some follow up patches I would like to do. 16:51:37 Like using the output of "tox -e gendriverlist" to make this sane: http://docs.openstack.org/developer/cinder/drivers.html 16:51:49 tbarron: bluex is the person to ask on status. Not sure when he'll be online. 16:51:50 Not even sure what the page is supposed to be conveying. 16:51:54 smcginnis: I think interface checks would be good 16:52:06 dulek: ty! 16:52:17 smcginnis: Your demo at the summit showed how useful they can be. 16:52:25 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 diablo_rojo: Cool, glad it at least made some sense. :D 16:53:00 smcginnis: Seemed like a pretty easy way to get tons of info about all the drivers 16:53:10 smcginnis: I've been meaning to get to that patch, will try harder 16:53:24 smcginnis: IT certainly seems to achieve good things 16:53:51 I'm thinking so, but I certainly have a biased view. ;) 16:55:11 Anything else? 16:55:35 K, thanks everyone. 16:55:39 Thanks. 16:55:45 #endmeeting