15:00:17 #startmeeting manila 15:00:17 Meeting started Thu Dec 7 15:00:17 2017 UTC and is due to finish in 60 minutes. The chair is bswartz. Information about MeetBot at http://wiki.debian.org/MeetBot. 15:00:18 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 15:00:21 The meeting name has been set to 'manila' 15:00:27 hello o/ 15:00:30 hello folks 15:00:31 hi 15:00:33 hello 15:00:58 hi 15:01:03 * bswartz looks around for more people 15:01:18 courtesy ping: toabctl xyang markstur vponomaryov cknight 15:01:46 #topic announcements 15:01:50 toabctl is on paternity leave 15:02:01 hello o/ 15:02:02 I know -- but he still gets a courtesy ping 15:02:20 I just have a list of the core reviewer team 15:02:27 bswartz: ack 15:02:33 Today is the Queens-2 milestone 15:02:46 dustin is having irc connect to freenode issues 15:02:55 tbarron: :-( 15:03:29 I plan to tag the releases this afternoon 15:03:40 awesome 15:03:56 unlike last milestone where I planned to tag them and then promptly forgot :-[ 15:03:56 hi 15:04:07 tbarron, I'm back :) 15:04:10 toabctl: welcome back! 15:04:20 toabctl: nice! 15:04:23 toabctl: hope you're enjoying parenthood 15:04:29 but I'll be away for another 3 month next year :) 15:04:33 I did! 15:04:35 thanks 15:04:53 #agenda https://wiki.openstack.org/wiki/Manila/Meetings 15:05:01 toabctl: well, double congrats 15:05:17 okay we have a few topics today 15:05:22 #topic Rocky PTG 15:05:34 #link https://etherpad.openstack.org/p/manila-rocky-ptg 15:05:44 #link https://www.openstack.org/ptg/ 15:05:54 Planning for this event is underway 15:06:09 We should have discussed this last week but it slipped my mind 15:06:35 I have already told the foundation that Manila does plan to participate in the PTG in Dublin 15:07:35 I'm open to hearing alternative opinions, but my own opinion is that not participating in the Queens PTG was a mistake 15:07:51 o/ 15:08:07 o/ 15:08:11 o/ 15:08:11 and enough of you who I've talked to agreed, so I went ahead and said yes 15:08:19 o/ 15:08:27 but I wanted to discuss it now, in case anyone prefers the virtual format 15:08:44 because it's early enough that we could in theory change our plans 15:09:20 in either case, time zone issues will make it hard for some people to attend 15:09:43 Cinder team will be doing the PTG. Just FYI. 15:09:44 Meeting in Dublin at least gives people the option to avoid that problem by traveling 15:09:54 jungleboyj: +1 15:10:07 jungleboyj: do you happen to know which days cinder will be meeting yet? 15:10:48 bswartz: Requested the usual Wed-Fri 15:10:55 do we have a preliminary schedule or know the days for manila? 15:10:58 Though we might be able to get by with just 2 days. 15:11:18 jungleboyj: my preference would be for Manila to meet Tuesday+Friday 15:11:44 In Denver, Friday was a pretty empty day in the Cinder room 15:12:02 bswartz: Yeah, that was what I was thinking. Sounds like a reasonable plan. 15:12:24 I'm not sure they'll give us a room on Tuesday, given how things went in Denver, but that's what I'm pushing for 15:12:40 anyways, it's important that people who know they are coming add their name to the etherpad so we can get a count 15:12:53 bswartz: do we know what cross-project meetings are scheduled for Tues yet? 15:13:06 if you want to come, but you're unsure about travel budget, I'd like to know that 15:13:13 tbarron: no idea 15:13:33 tbarron: I'd rather conflict with crossproject stuff than cinder though 15:13:43 and I presume the most important crossproject stuff will get covered on Monday 15:13:54 * jungleboyj crosses fingers 15:14:48 Well, we'll probably do some running in and out of groups no matter what so Tues/Friday seems reasonable. 15:14:53 okay moving on... 15:15:02 #topic How should we fix "shares cannot be found by name in admin context" 15:15:13 zhongjun: this is your topic 15:15:22 bswartz: thanks 15:15:59 I just want to check if it is a right way to fix this bug 15:16:04 The share that created by non-admin cannot be found by share name in admin context 15:16:04 Because we cannot list all shares (belong to other non-admin project) by admin in manila client, (currently we doesn’t add {all_tenants: 1} parameter in list all share api ). so we cannot filter the share by share name. 15:16:41 But if we add {all_tenants: 1} to common place that we will list all resources in client, there is another problem 15:16:41 We cannot create a share with share network name by non-admin. Because {all_tenants: 1} parameter only work for admin. So we cannot get the share network by share network name when we create a share with share network by non-admin. 15:17:06 So, jiaopengjun try to change the share network policy from admin_api to default. Like the share type does 15:17:19 okay I'm a bit confused 15:17:29 I know there are issues with using the CLI as admin 15:17:42 but I didn't think the admin had problems filtering by name 15:17:50 * dustins finally gets into freenode 15:18:01 dustins: huzzah! 15:19:02 Yeah, something strange is going on with it, I still can't connect through Hexchat (using Webchat now) 15:19:05 bswartz: dustin confirmed that bug that arnewiebalck reported 15:19:19 so who knows what the underlying issue is 15:19:22 ? 15:19:41 bswartz: the admin didn't have the problem filter by name with API, but it has problem filter by name with CLI 15:19:42 admin couldn't find shares belonging to other tenants by name 15:19:47 Administrators aren't able to use share filters to get objects outside of their own project 15:19:47 we don't try to ensure uniqueness of names do we? 15:20:14 no, it's that we weren't using 'all-tenants' in the search 15:20:20 gouthamr: that's an implementation choice we could easily change though, isn't it? 15:20:28 which you really don't want to do for non-admins 15:20:33 is there a performance reason we don't allow this? 15:20:53 bswartz: I think it was just accidental, not deliberate. 15:20:54 bswartz: no, in client, we just list all shares, then filter by name in those shares 15:20:58 --all-tenants can be cripplingly slow in a large cloud 15:21:16 cinder allows it and cinder performs very well, right jungleboyj? 15:21:20 * tbarron ducks 15:21:25 * bswartz chuckles 15:21:56 :-) Yeah ... 15:22:11 so our sqlalchemy layer isn't smart enough to do filtering on the DB side? 15:22:24 bswartz: agree this is an implementation issue, it feels like the policy should be enforced on using "all_tenants" 15:23:28 I think the performance issue wasn't a motivator for us implementing what is really a bug. 15:23:42 We could address the perf issue separately after fixing the functionality 15:23:50 as long as admins are allowed to view all the shares using --all-tenants, then they should be allowed to filter that way too -- my main concern is that we should try to implement filtering in an efficient way so that listing a few shares by name is not exactly as slow as listing all the shares 15:23:55 you don't need to check for context.is_admin and all_tenants, you only need to enforce policy on the usage of "all_tenants", that way, if all_tenants is specified, we'll not limit the search to the project 15:24:27 yeah I agree 15:24:28 we allow all-tenants for other admin context searches 15:24:39 so here are the links 15:24:45 #link https://bugs.launchpad.net/manila/+bug/1721787 15:24:46 Launchpad bug 1721787 in python-manilaclient "Shares cannot be found by name in admin context" [Medium,In progress] - Assigned to jiaopengju (pj-jiao) 15:24:47 maybe lots of them could have perf issues, so we should look at that too 15:24:50 #link https://review.openstack.org/#/c/522607/ 15:24:56 #link https://review.openstack.org/#/c/522452/ 15:25:28 I don't understand 522607 15:25:53 why should *non* admin users use all_tenants: 1 ? 15:26:27 zhongjun: can you explain why there are 2 patches here? 15:26:40 should we just be looking at one of them? or both? 15:26:40 522452 appears to at least frame the issue correctly 15:26:47 one of the client patch 15:27:03 zhongjun: do you believe both are needed? 15:27:28 one of the client patch -- just add the all_tenants in common place to list all resources 15:27:49 It could have a better way to solve it 15:28:24 yeah the client patch seems odds to me 15:28:32 maybe 522607 just has a confusing commit msg 15:28:38 but he was follow the cinder method to fix it 15:28:55 s/odds/odd/ 15:29:04 issue is do we fix this client side or server side, right? 15:29:26 well no matter what, the server has to allow the admin to do what is needed 15:29:34 just fix this client side is not enough now 15:29:43 so far I'm not sure if the thing preventing correct behavior is server or client side 15:30:01 but I'm pretty foggy on how policies are enforced 15:30:11 I don't know why there is a client-side policy at all 15:30:33 err wait 15:30:42 I got the changes mixed up 15:32:18 so if I read this correctly cinder has a hack: by policy it allows non-admin users to try to get resources with {all_tenants: 1} and then *in the code* it checks the context and if it's not admin just ignores that setting 15:32:20 weird 15:32:24 okay well we need to code review these 2 changes and make sure they're not doing anything that harms security or performance, but I think we agree in principle that we want to allow this, and it's okay if it's suboptimal performance as long as it doesn't make things worse 15:32:38 I guess it was an early implementation of "policy in code" 15:32:40 :) 15:33:06 we better move on to the next agenda item 15:33:08 i don't agree with the bug 15:33:10 :( 15:33:16 tbarron: non-admin users just skip the all_tenants:1 , then list the all shares that belong to non-admin 15:33:16 gah! 15:33:26 gouthamr: what behavior do you expect? 15:33:29 tbarron: I was looking around that code the other day and wondered what was going on. 15:33:31 i think the behavior right now is correct 15:33:36 gouthamr: please explain why in the bug! 15:33:42 yes, will do 15:33:47 gouthamr: why shouldn't admins be able to filter by name? 15:34:02 admins can filter by name, but they should use "all-tenants" explicitly 15:34:11 gouthamr: why cannot filter by name? 15:34:13 manila list --name~ test --all-tenants 15:34:21 does that work correctly today? 15:34:28 yes, just tested 15:34:41 oh 15:34:48 gouthamr: wow 15:34:50 ok, so let's see what arne says back when you post that in the bug 15:35:07 maybe he'll go raise a bug on cinder, he works in both projects 15:35:08 #action gouthamr respond on https://bugs.launchpad.net/manila/+bug/1721787 15:35:10 Launchpad bug 1721787 in python-manilaclient "Shares cannot be found by name in admin context" [Medium,In progress] - Assigned to jiaopengju (pj-jiao) 15:35:10 :D 15:35:15 is arnie just wants the client to implicitly send --all-tenants when he runs as admin? 15:35:26 s/is/so/ 15:35:46 I don't know that he considered the possibility that gouthamr presents; let's see. 15:35:53 yeah... 15:36:13 I don't like the idea of implicitly setting a flag with performance-crippling implications 15:36:25 +1 15:36:48 I'd prefer a way to maybe set an environment variable that always sets --all-tenants if the admin really wants that 15:37:03 Maybe this was by design after all (contrary to my earlier remarks). 15:37:03 Agreed 15:37:16 +1 15:37:18 okay, thanks for reproducting the issue gouthamr 15:37:24 #topic Outstanding driver work? 15:37:26 thanks gouthamr 15:37:28 tbarron: you're up 15:37:56 infinidat driver is about ready to merge, Jun and Goutham will need to re-review 15:37:57 #link https://review.openstack.org/#/c/515432/ 15:38:03 #link https://review.openstack.org/#/c/510547/ 15:38:04 onit 15:38:18 cephfs-nfs ha changes are ready to review again 15:38:26 I had +2 and Jun had +1 15:38:41 rraja just submitted again with changes in light of 15:38:43 and leave a few questions :) 15:38:44 Jun's remarks 15:38:53 it needs to go through Jenkins. 15:39:06 Would be good to merge these before M2 tag is cut. 15:39:22 another one that was being worked on is: https://review.openstack.org/#/c/465846/ - I don't know if the author intended to get this through in Queens 15:39:23 Will make integration in distros, etc. a lot easier. 15:39:23 tbarron: how likely is that? 15:39:52 bswartz: if you don't tag till late afternoon and we can get reviewers it seems likely 15:40:06 I don't intend to hold up the release for something late 15:40:13 But we need eyes 15:40:26 but if it's getting workflowed soon then it could make it anyways 15:40:38 I don't think these are late, we just need reviewer cycles and those are scarce. 15:40:59 * bswartz is reviewing ensure share 15:41:07 That is, these have been up, passing CI, etc. for some time. 15:41:31 bswartz: thanks for reviewing ensure share :) 15:41:44 when we set the driver deadline to be the week of milestone-2, the intention was not to merge them by milestone-2 15:42:04 gouthamr: infortrend review is not passing CI 15:42:05 just to make sure they get in by milestone-3 without stealing last-minute review cycles 15:42:14 bswartz: ack 15:42:48 so let me put it this way: I'll do my best to get the infinidat one merged now because it is ready. 15:42:51 tbarron: infortrend never passed CI? 15:42:54 cool 15:43:08 I think the cephfs one is ready too and would appreciate if reviewers could help it merge. 15:43:14 and I haven't checked, but is it safe to assume that cephfs-nfs has working CI? 15:43:18 zhongjun: I don't know about never. 15:43:32 bswartz: yes, and the CI is now exercising the new aspects. 15:43:45 great 15:43:52 bswartz: it's not a new driver, but it's an important change. 15:44:08 #topic Tag M2? 15:44:13 tbarron: this is your topic too 15:44:21 you already addressed it 15:44:35 okay I didn't know if you had anything specific to discuss here 15:44:52 I guess we should ask if there's anything else that should merge first. 15:44:59 I'll aim to do this fairly late in the day 15:45:07 I'd put your latest ipv6 fix in that category but it just merged. 15:45:52 I'd like to get the policy-in-code stuff merged but honestly I think there are too many patches to do them all today. 15:46:30 We merged "share type with description" in manila project, but we didn't merged the client side, is it affect about M2? 15:46:31 It looks good to me but I haven't reviewed them in detail. We should set a goal for that though for reviewers . 15:46:55 +1 need some more time to test/review policy-in-code 15:46:58 M2 is a bit of an artificial deadline. 15:47:12 yeah I won't be sad if something doesn't make the milestone 15:47:33 I would prioritize bugfixes personally 15:47:48 and aim for the content to all be in by Q-3 15:47:57 (that's queens-3 not quarter-3) 15:48:04 :) 15:48:10 Looking past M2 to (potentially) break 3rd party CIs with https://review.openstack.org/#/c/512300/ 15:48:34 in any case I'll hold off on tags until later today, so you have at least 4-5 hours to workflow stuff 15:48:51 gouthamr: how is that one going from your perspective? Did you have a chance to test it w/ Netapp? 15:48:58 and I'll check that the gate pipeline is empty so as not to miss anything in flight 15:49:06 tbarron: yes 15:49:22 * bswartz trying to save some time for dustins topic... 15:49:26 tbarron: and i don't think we need to break the 3rd party CIs, they all need a one line change if using devstack-gate 15:49:38 that can work right now, even without that patch 15:49:59 #topic Let's go over new bugs 15:50:04 gouthamr: ack, we'll talk to raissa bout sending out an email 15:50:15 dustins: do you have anything for today? 15:50:25 #link https://etherpad.openstack.org/p/manila-bug-triage-pad 15:50:26 bswartz: I've got a few bugs! 15:51:02 We can scream through them if we'd like 15:51:08 tbarron: if not using devstack-gate, they'll need to clone the repo and install it in $DEST/manila-tempest-plugin prior to DevStack - the rest will just work fine 15:51:31 gouthamr: you'll need to show me how to do that in my dev env 15:51:51 I run the plugin from the manila repo still 15:51:58 dustins: what's up first? 15:52:20 bswartz: +1 a devref update is in order 15:53:44 https://bugs.launchpad.net/manila/+bug/1736310 15:53:45 Launchpad bug 1736310 in Manila "Invalid or unsupported share access level ro." [Undecided,New] - Assigned to junboli (junboli) 15:54:15 Oh, I told to junboli, it isn't a bug 15:54:41 #link: https://review.openstack.org/#/c/161176/ 15:54:53 Okay! I just marked it as "Invalid" then 15:54:57 thanks, zhongjun! 15:55:17 https://bugs.launchpad.net/manila/+bug/1733286 15:55:18 Launchpad bug 1733286 in Manila "snapshot share data Sync with the source share" [Undecided,New] 15:55:29 dustins: np 15:55:45 dustins: maybe put that link in the bug 15:56:00 wow this is a lot of info 15:56:25 this is a weird one, I talked with him on irc 15:56:43 and kept asking for more info b/c I couldn't believe what he was seeing 15:57:08 apparently with generic driver and current share server image the exports are 15:57:10 so manila snapshots are missing some data that was present at the time of the snapshot? 15:57:18 is that the essence of this bug? 15:57:26 tbarron: Done, I added the link the bug 15:57:34 on the share server it all looks correct 15:57:59 tbarron: I could easily imagine VFS caching resulting in this kind of problem 15:58:04 on the client the share created from snapshot has content from the orignal share after 15:58:10 IIRC the generic driver doesn't attempt to flush caches before taking snapshots 15:58:15 the original share has its content changed 15:58:55 making changes to the source share affects shares created from a snapshot of the source share 15:59:07 oh that's different 15:59:20 well step 1 is to reproduce the issue on master 15:59:27 i.e. create original share; put 'hello world in it'; take snapshot; create share from snapshot; change origiinal share to 'bye bye cruel world'; look at share created from snapshot and see 'bye bye cruel world' 15:59:36 if it still exists, then obviously we should figure out why and fix it 15:59:42 I see there's no owner 15:59:52 but if you look at the second share on the share server it is still 'hello world' 15:59:52 normally I would jump on a bug like this, but I'm pretty busy with my other changes 16:00:41 and we're out of time 16:00:56 dustins: were there any other important bugs we need to discuss today? 16:01:06 we could hop over the channel if so 16:01:15 A few short ones that have to do with automatically created docs bugs 16:01:26 either way, we're done with this meeting 16:01:27 But we can take that to the channel :D 16:01:32 thanks all! 16:01:38 #endmeeting