16:00:37 <jungleboyj> #startmeeting cinder
16:00:37 <openstack> Meeting started Wed Mar  6 16:00:37 2019 UTC and is due to finish in 60 minutes.  The chair is jungleboyj. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:00:38 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:00:40 <openstack> The meeting name has been set to 'cinder'
16:00:45 <whoami-rajat> Hi
16:00:48 <rajinir> hi
16:00:52 <geguileo> hi o/
16:00:55 <walshh_> hi
16:00:59 <woojay> Hello
16:01:01 <rosmaita> o/
16:01:14 <eharney> hi
16:01:29 <smcginnis> o/
16:01:53 <e0ne> hi
16:01:56 <jungleboyj> @!
16:01:56 <_pewp_> jungleboyj ( ･ω･)ﾉ
16:02:14 <jungleboyj> smcginnis:  You should come join me in Scottsdale before the next Snowmageddon.
16:02:41 <yikun> Hello
16:02:50 <smcginnis> That would be nice.
16:03:09 <jungleboyj> :-)  Next year you can come join our guys' week for Spring Training.  :-)
16:03:11 <enriquetaso> o/
16:03:29 <jungleboyj> smcginnis:  Wondering if I am going to have issues getting home Sunday though.  Ugh.
16:03:57 <jungleboyj> Ok.  Lets get started.
16:04:15 <jungleboyj> courtesy ping:  jungleboyj diablo_rojo, diablo_rojo_phon, rajinir tbarron xyang xyang1 e0ne gouthamr thingee erlon tpsilva ganso patrickeast tommylikehu eharney geguileo smcginnis lhx_ lhx__ aspiers jgriffith moshele hwalsh felipemonteiro lpetrut lseki _alastor_ whoami-rajat yikun rosmaita enriquetaso hemna _hemna
16:04:37 <xyang> hi
16:04:39 <jungleboyj> #topic announcements
16:04:51 <jungleboyj> PTL Nominations are now open.
16:05:02 <tbarron> hi
16:05:09 <jungleboyj> I have been PTL for a while now.
16:05:32 <jungleboyj> I am willing to continue but don't want to hold anyone up from running if they are interested in doing so.
16:05:57 <e0ne> jungleboyj: thank you for your hard work!
16:06:10 <jungleboyj> We are kind of at that point where Sean was, when I ran, that I am happy to continue but also think change is good for the team.
16:06:40 <jungleboyj> My time commitments keep getting to be more stretched and I don't want anyone to hesitate if they feel like they could do better.  :-)
16:06:45 <jungleboyj> e0ne:  Thank you!
16:06:52 <jungleboyj> It is a team effort.
16:07:07 <rosmaita> what e0ne said
16:07:50 <_pewp_> hemna (;-_-)ノ
16:07:50 <jungleboyj> rosmaita:  Thanks.
16:08:03 <jungleboyj> hemna:  Welcome.
16:08:14 <jungleboyj> Does anyone have questions about that?
16:08:49 <jungleboyj> If anyone is interested or has questions feel free to reach out to me.
16:09:25 <jungleboyj> My other announcement is we are now at Milestone-3 for Stein.
16:09:38 <jungleboyj> #link https://releases.openstack.org/stein/schedule.html
16:10:08 <jungleboyj> Now is the time to transition from development to more testing.  No more merging new features.
16:10:45 <e0ne> do we have anything for FFE?
16:10:57 <jungleboyj> We should try to get bugs addressed and merged.
16:11:05 <jungleboyj> e0ne:  Not that I have seen.
16:11:56 <jungleboyj> Anyone have comments or questions there?
16:12:18 <rosmaita> i have a patch up for a requirements change, does that have to merge this week?
16:12:29 <rosmaita> https://review.openstack.org/#/c/641037/
16:12:39 <jungleboyj> rosmaita:  I don't believe that has to merge but sooner is better.  Let me look here.
16:12:45 <rosmaita> eharney has one also: https://review.openstack.org/#/c/641065/
16:12:51 <rosmaita> i didn't notice any others
16:13:10 <eharney> the schedule says req freeze at m-3
16:13:12 <jungleboyj> rosmaita:  Oh, this is redrobot requirements freeze.
16:13:14 <jungleboyj> So yes.
16:13:32 <jungleboyj> *requirements freeze
16:13:43 <jungleboyj> We can take a look at those and get them merged.
16:13:49 <rosmaita> ok, cool
16:13:52 <smcginnis> That taskflow one might be good to get in quickly to watch for side effects.
16:14:01 <jungleboyj> smcginnis:  ++
16:14:01 <e0ne> smcginnis: +2/A
16:14:10 <smcginnis> e0ne: ++
16:14:34 <rosmaita> great, thanks everyone
16:14:36 <jungleboyj> Anything else that people want to discuss around M-3?
16:15:34 <jungleboyj> Ok.  Cool.  Moving on.
16:15:56 <jungleboyj> General filtering API modification
16:16:03 <jungleboyj> #topic General filtering API modification
16:16:06 <jungleboyj> whoami-rajat:
16:16:09 <whoami-rajat> Hi
16:16:27 <jungleboyj> @!
16:16:27 <_pewp_> jungleboyj |。･ω･|ﾉ
16:16:39 <whoami-rajat> So i've been digging around the general filtering API and it's bugs regarding non-admin users
16:16:48 <jungleboyj> whoami-rajat:  Cool.
16:17:06 <whoami-rajat> i would like to get some feedback on my findings.
16:17:19 <jungleboyj> Ok.
16:17:33 <whoami-rajat> previously we used to pop out the invalid filters with a debug msg
16:17:44 <whoami-rajat> after the filtering API we error out on them
16:18:21 <whoami-rajat> since the filters mentioned in the resource_filters.json file are just a fraction of what all filters we support like is_public, all_tenants ...
16:18:42 <whoami-rajat> we're facing a lot of unexpected errors for non-admins
16:18:54 <eharney> all_tenants is a special case compared to most of these other filters IMO
16:19:15 <whoami-rajat> eharney:  cause all_tenants is common to multiple commands,
16:19:35 <whoami-rajat> is_public is also an issue related to volume types
16:19:53 <smcginnis> We should probably go back to existing behavior of just logging it. But I can also see why we wouldn't want to ignore something a client has requested to filter on.
16:20:25 <jungleboyj> Don't want to spam the logs.
16:21:09 <eharney> well, it makes sense to distinguish between filters that are valid vs ones that will never work, as well as returning permission denied when appropriate (denied by policy etc)
16:21:28 <jungleboyj> eharney:  Agreed.
16:21:40 <jungleboyj> whoami-rajat:  Why are there so many errors for non-admins?
16:22:02 <eharney> so i'm pretty sure the answer isn't to stop throwing errors
16:22:04 <whoami-rajat> eharney: also an interesting case that may fail commands using all_tenants the revert-to-snapshot was failing because we first search for snapshots and all_tenants is automatically appended in the search_opts, so it should have generated NotFound error but generating BadRequest due to filtering API.
16:22:27 <hemna> jungleboyj: would like to get my patch in for the new driver options discovery
16:22:32 <hemna> it's been sitting a few days
16:22:50 <whoami-rajat> jungleboyj: because while adding the support for general filtering, all filters weren't covered.
16:22:52 <jungleboyj> hemna:  Hold on.
16:23:00 <eharney> that could be fixed in the client, or we could handle all_tenants in a way that the client expects/expected
16:23:26 <eharney> but all_tenants is more about who has privileges to see what than resource filtering, afaict
16:23:47 <whoami-rajat> jungleboyj: since the filters are only for non-admins (admins are allowed for mostly all filters), this caused failure in multiple commands after 3.31 mv
16:24:41 <jungleboyj> Ok.
16:24:52 <whoami-rajat> eharney: yes, popping out invalid filters also doesn't look a good approach as if client sends it intentionally we shouldn't process the request
16:24:56 <hemna> we are seeing this problem as well
16:25:28 <whoami-rajat> eharney: one solution could be to add all valid filters in the json but that would cause a lot of duplicacy
16:25:33 <eharney> whoami-rajat: i would agree for filters in general, because we designed the server to reject invalid filters.  but, the client was clearly designed before that to assume that all_tenants will work, and i think it's reasonable to treat that one separately
16:25:53 <hemna> do we have a bug for this in lauchpad
16:25:54 <hemna> ?
16:26:12 <jungleboyj> Sounds like this is a bug that should be tracked.
16:26:39 <eharney> yeah, but it's not just a small bug, it's an area we need to actually document the design of because nobody is sure how this is supposed to work
16:26:59 <hemna> another good reason for a bug :)
16:27:08 <whoami-rajat> eharney: i tried to handle the case for all_tenants but then it error out for is_public, i think there may occur multiple other scenarios as well
16:27:26 <jungleboyj> :-)  Just to take a step back.  This is a big bug but it is a nuisance right, not a major issues.
16:27:28 <whoami-rajat> hemna: i found multiple different bugs for different commands,
16:27:57 <hemna> but it's a filter permissions issue no?
16:27:58 <eharney> it's a couple of different bugs, the server is wrong, the client is also doing things a bit wrong
16:28:01 <eharney> no
16:28:07 <hemna> we have a demo account that can't list volumes
16:28:19 <jungleboyj> :-(
16:28:30 <hemna> when filters are added to that list command issued
16:28:43 <hemna> like....status=available
16:28:55 <whoami-rajat> I think the idea to implement it was to avoid multiple checks for each filter and stack them at one place.
16:29:20 <whoami-rajat> but we forgot to cover all filters possible and only added the major ones
16:29:49 <eharney> forgot to cover what exactly?  isn't the list of filters configurable?
16:30:05 <jungleboyj> whoami-rajat:  How hard would it be to fix this by adding the filters to the json file as a bandaid and then work on the long term fix after that?
16:31:13 <whoami-rajat> eharney:  yes,
16:31:42 <whoami-rajat> jungleboyj:  depends on every resource. i can start with it if that is the intended approach.
16:31:51 <eharney> adding all_tenants to the default filter list is wrong IMO
16:32:01 <hemna> hehe
16:32:03 <hemna> yah
16:32:14 <jungleboyj> eharney:  That sounds like a special case.  Right?
16:32:44 <jungleboyj> We should at least handle the non-admin filter cases by default.
16:32:47 <whoami-rajat> eharney: probably needs to be added in every resource,
16:32:53 <hemna> a workaround would be nice for short term, but this seems quite broken in general
16:33:00 <eharney> whoami-rajat: what does?
16:33:04 <jungleboyj> hemna:  Agreed.
16:33:19 <whoami-rajat> eharney: all_tenants
16:33:23 * jungleboyj sadly doesn't understand this code path so well.
16:33:39 <eharney> i don't think it has to be added to every resource
16:34:00 <eharney> what i suggested a couple weeks ago was that we special case all_tenants and verify that it is handled as expected at a layer further into the stack, which it should be
16:34:39 <jungleboyj> all_tenants are the things that all_tenants should be able to filter upon?
16:34:45 <eharney> no
16:34:49 <jungleboyj> Ugh.
16:34:57 <eharney> it's an arg that says "show me resources owned by other tenants"
16:34:59 <whoami-rajat> eharney: IIUC you're suggesting we should pass it through the filtering and then handle this particular as a special case?
16:35:14 <eharney> and it is specifically checked all over our code with checks like "if is_admin and all_tenants..."
16:35:17 <hemna> that's admin like privs
16:35:19 <eharney> because it's not a normal filter
16:35:27 <eharney> it's just caught by the generic filtering code
16:35:31 <eharney> because it wasn't written right
16:35:35 <eharney> and nobody noticed
16:35:35 <whoami-rajat> jungleboyj: it's an admin action to show resources in every project
16:35:45 <eharney> so we should special case it in the filtering code and do it correctly, like it used to work
16:36:32 <whoami-rajat> eharney: i checked it with the previous code, it just pops out the all_tenants and proceeds normally. so a special case would be good.
16:37:27 <whoami-rajat> eharney: and other filters causing issues could be directly added to resource_filters file.
16:37:28 <jungleboyj> eharney:  Ok, that makes sense.
16:37:51 <eharney> whoami-rajat: that just depends on what we want to allow by default vs leave up to deployers to enable
16:38:02 <eharney> whoami-rajat: we probably don't want to just add everything
16:38:22 <whoami-rajat> eharney: also i tried handling that in clients for the findall resource method https://review.openstack.org/#/c/641310/
16:38:39 <jungleboyj> eharney:  Oh, this was added for the read-only admin case.  Right?
16:38:57 <eharney> jungleboyj: what was?
16:38:58 <whoami-rajat> eharney: no, not all, just some important ones like is_public for volume types.
16:39:11 <jungleboyj> Adding all_tenants
16:39:23 <eharney> i think all_tenants has always been there
16:39:56 <jungleboyj> Hmmm.  Ok.
16:40:02 <jungleboyj> I will just stay out of the way.
16:41:51 <jungleboyj> It sounds, however, like we should special case it and set some reasonable set of defaults to avoid spamming the logs.
16:42:22 <jungleboyj> The the defaults aren't right for the end user they will need to update accordingly.
16:42:57 <eharney> it looks like we can just throw away or ignore "all_tenants" in reject_invalid_filters if the caller isn't admin
16:42:59 <eharney> yeah
16:43:44 <jungleboyj> Cool.  That makes sense.
16:43:52 <whoami-rajat> eharney: but when a non-admin user intentionally passes all-tenants, is it ok not to error out?
16:44:20 <eharney> we used to just ignore it so it didn't do anything
16:44:30 <eharney> i don't see why we couldn't decide to start doing that again
16:44:55 <smcginnis> Especially if all-tenants:false
16:45:09 <jungleboyj> That makes sense.
16:45:40 <whoami-rajat> smcginnis that we used to ignore in client
16:45:56 <whoami-rajat> so we want to ignore that in server now?
16:46:09 <eharney> we should fix the client to not send the arg when it isn't appropriate, that's the easier part to fix though
16:46:48 <smcginnis> Easier for the python client. There are a lot of other clients out there though, so handling this sanely on the server side is the best option.
16:47:17 <eharney> we should fix both, the client shouldn't be sending all_tenants=0 for no reason
16:47:29 <eharney> but the server fix is the more important for sure
16:48:14 <whoami-rajat> eharney:  smcginnis  ok. this seems to be the better workaround to fix at both places.
16:48:21 <jungleboyj> whoami-rajat:  Yep.
16:48:47 <whoami-rajat> will do
16:49:11 <jungleboyj> eharney:  Thank you for the input.
16:49:47 <whoami-rajat> jungleboyj: eharney smcginnis thanks for all the useful feedback.
16:49:49 <jungleboyj> whoami-rajat:  Know what to do?
16:50:02 <whoami-rajat> jungleboyj: yes.
16:50:16 <jungleboyj> whoami-rajat:  Awesome!  Thank you for all the work you have been doing.
16:50:39 <jungleboyj> #action whoami-rajat  to propose fixes to handle all_tenants properly on both the client and server sides.
16:51:49 <jungleboyj> Ok.  So, I think that is all on that topic.
16:52:11 <jungleboyj> Last topic.  rosmaita  Are those the requirement changes you proposed earlier?
16:52:30 <rosmaita> looking
16:52:36 <eharney> yes
16:53:02 <whoami-rajat> jungleboyj: np :) thanks.
16:53:17 <jungleboyj> Ok.  So we already covered that.
16:53:21 <rosmaita> jungleboyj: what eharney said
16:53:24 <jungleboyj> #topic open discussion
16:53:53 <jungleboyj> Anything else that needs to be discussed?
16:54:21 <jungleboyj> Oh, reminders to put topics in for the forum.
16:54:23 <eharney> https://review.openstack.org/#/c/641414/ is a couple of nice reverts for someone to review
16:54:47 <jungleboyj> We seriously can't have nothing but what I proposed to talk about there:
16:54:56 <jungleboyj> #link https://etherpad.openstack.org/p/cinder-denver-forum-brainstorming
16:55:11 <jungleboyj> Are those submissions due tomorrow, of Friday.
16:55:23 <jungleboyj> Friday, so I will propose what is there.
16:55:28 <jungleboyj> Please add topics
16:55:38 <jungleboyj> hemna:  You had something.
16:56:17 <whoami-rajat> i think raghavendra has a patch for HPE 3par driver, he has been pinging continuously mail and IRC for the review. https://review.openstack.org/#/c/634119/
16:57:11 <hemna> oh hey
16:57:24 <hemna> you mentioned feature freeze
16:57:30 <jungleboyj> whoami-rajat:  Need eharney and hemna  To look at the responses there.
16:57:33 <jungleboyj> hemna:  Yeah.
16:57:44 <hemna> so I wanted to ask for some love on my feature patch for driver options
16:57:52 <jungleboyj> hemna:  Yeah.  Link?
16:57:53 <eharney> i'm still skeptical about that 3par patch
16:58:01 <hemna> https://review.openstack.org/#/c/635255/
16:58:20 <hemna> https://review.openstack.org/#/c/639193/
16:58:22 <hemna> those 2 are related
16:58:27 <hemna> one is for cinder
16:58:30 <hemna> and the other is for cinderlib
16:58:52 <jungleboyj> hemna:  Ok.  I will try to get my love on with those.
16:59:05 <hemna> thanks
16:59:54 <hemna> fwiw, you can see all of the driver options in the drivers list docs now with that patch
16:59:57 <hemna> http://logs.openstack.org/55/635255/10/check/openstack-tox-docs/0200672/html/drivers.html
17:00:16 <hemna> and I ordered all the drivers alphabetically
17:00:40 <jungleboyj> Nice!
17:00:40 <hemna> and put the unsupported drivers at the bottom
17:00:59 <jungleboyj> Ok.  Will definitely take a look at the patch.
17:01:04 <jungleboyj> Ok, we are out of time.
17:01:08 <jungleboyj> Thanks everyone!
17:01:14 <jungleboyj> #endmeeting