13:00:09 <alex_xu> #startmeeting nova api
13:00:10 <openstack> Meeting started Wed Dec 14 13:00:09 2016 UTC and is due to finish in 60 minutes.  The chair is alex_xu. Information about MeetBot at http://wiki.debian.org/MeetBot.
13:00:12 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
13:00:14 <openstack> The meeting name has been set to 'nova_api'
13:00:21 <alex_xu> who is here today?
13:00:32 <gmann> o/
13:01:00 <Kevin_Zheng> o/
13:01:25 <sdague> o/
13:01:49 <alex_xu> first, let me give a quick update before we jump in the filters....
13:02:08 <alex_xu> #link https://etherpad.openstack.org/p/ocata-nova-priorities-tracking
13:02:16 <johnthetubaguy> o/
13:02:28 <alex_xu> the server side patch of the deprecation of image-metadata is merged
13:03:00 <alex_xu> and I feel the pagination of single_tenant_usage is close
13:03:03 <alex_xu> #link https://review.openstack.org/386093
13:03:54 <alex_xu> so we have better give more attension for other api patches, hope we can merge them all
13:04:09 <alex_xu> #topic API Priorities
13:04:32 <alex_xu> so....i think we can begin to talk about filters again. I have two points for it today
13:05:10 <alex_xu> first is about the policy rule, sdague have concern on it, it make the API behaviour different without any discoverable way.
13:05:35 <alex_xu> as I remember johnthetubaguy have point that the user don't want to remove it?
13:06:00 <johnthetubaguy> I am worried operators rely on those, given their comments at the summit
13:06:30 <johnthetubaguy> the policy idea was a way to relax the restrictions in some clouds where its really wanted
13:06:47 <johnthetubaguy> (with a view towards capabilities API fixing discoverability eventually)
13:07:22 <sdague> johnthetubaguy: did we get any comments about which filters people were concerned about?
13:07:26 <alex_xu> yea, I +1 for that also. but I can't answer the question of behaviour change
13:07:38 <sdague> I *really* don't want to make this a config option
13:07:58 <sdague> I would much rather add in things people are concerned about being dropped
13:08:16 <alex_xu> emm....maybe we should discussion the second problem first. That may change which filters we want to remove
13:08:18 <johnthetubaguy> sdague: basically no, they seemed to want everything for admins
13:08:24 <alex_xu> s/discussion/discuss
13:08:28 <sdague> johnthetubaguy: define everything
13:09:19 <johnthetubaguy> sdague: well all API visible properties, filter and sorting, but I don't think we have that today, it wasn't really a useful data point I guess
13:09:43 <sdague> right
13:10:00 <sdague> here is the thing, we either decide we're going to define this, so the behavior is known
13:10:22 <sdague> or we decide we aren't, and then any db changes randomly break people with no warning in the future
13:11:06 <johnthetubaguy> right, I am on the define side of things, for sure, the status quo is stupid
13:11:23 <alex_xu> actualy only one db index can be used in the query....so I feel we can keep all the filters, due to most of them won't get benefit from index
13:11:25 <sdague> because exposing that policy point / config actually means we're saying "have raw access to our db... but no guaruntees that any of these things will keep working"
13:11:59 <sdague> alex_xu: yeh, without building a ton of indexes, that's a whole other challenge
13:12:04 <johnthetubaguy> but when it came to jay's worries about shrinking the set of allowed things, I was basically saying we have an always allowed set, and some extra we allow only for admins (but there is a policy to change that)
13:12:34 <sdague> johnthetubaguy: is that concern posted somewhere? still catching up.
13:12:36 <alex_xu> sdague: so that means the rest of those filters kept can't get benefit from index also. so keep one or keep all are no different
13:12:43 <johnthetubaguy> I don't believe anyone is arguing for unrestricted
13:13:18 <sdague> johnthetubaguy: ok, I guess I'm confused, what are you arguing for?
13:13:28 <johnthetubaguy> sdague: I thought that was in the comments in the spec, but unsure
13:13:50 <johnthetubaguy> so I should describe my preference here
13:14:21 <johnthetubaguy> first there is a set of things we allow for filter and sort, they should be limited to things that work today, and match properties we have in the API (ideally)
13:14:56 <johnthetubaguy> now, come the performance concerns, should we reduce the set further to those that don't cause big performance issues, a smaller set
13:15:09 <alex_xu> only the db index  'instances_project_id_deleted_idx' is used. So the result already shrink to single tenant, then even other filters can't get benefit from index it is fine
13:15:14 <johnthetubaguy> there are some that only make sense for admins (like project_id filters, or host filters)
13:15:38 <alex_xu> johnthetubaguy: the only bad one is admin with all_tenant=1
13:15:54 <sdague> ok, so lets see where we have common ground.
13:15:55 <alex_xu> we should encourge admin user list with all_tenant=0
13:16:09 <sdague> 1) agree that we should build this as a smaller set
13:16:20 <johnthetubaguy> +1
13:16:24 <sdague> well defined, matching the API for names (not random db tables)
13:16:28 <johnthetubaguy> +1
13:16:53 <alex_xu> emm...+1 for second one
13:17:14 <sdague> 2) all filters are indexed
13:17:19 <johnthetubaguy> (I guess the second one makes the first one true, but either way)
13:17:28 <sdague> my feeling on this is that I'm ok fixing this by adding more indexes
13:17:35 <sdague> not dropping filters
13:18:01 <sdague> I'm also ok punting on this, because it's a performance improvement that also gets odd if we use searchlight anyway
13:18:07 <gmann> but that way list can be smaller ?
13:18:08 <johnthetubaguy> so I am not sure (2) is actually required, I would say, we should remove end use DOS attack vectors
13:18:26 <johnthetubaguy> yeah, search light does confuse these things
13:18:41 <johnthetubaguy> s/ //
13:18:41 <sdague> johnthetubaguy: yeh, well dropping the join tables for query mitigates that
13:18:51 <sdague> the worst things are going to be removed
13:18:57 <johnthetubaguy> sdague: yeah, maybe thats enough
13:19:05 <sdague> then we're just doing filter / sort on unindexed values
13:19:07 <sdague> which sucks
13:19:19 <sdague> but it something a site can local patch if they see a ton of it
13:19:39 <sdague> 3) admin vs. non admin properties
13:19:39 <johnthetubaguy> so the big text columns, as I understand it, adding an index is more expensive to maintain than what it agains you, at least I hear that needs to be checked
13:19:53 <sdague> johnthetubaguy: sure, that makes sense
13:20:11 <sdague> how many bigtext columns are we thinking will be in the list here?
13:20:18 <johnthetubaguy> name?
13:20:18 <sdague> I hope we mostly pulled them out
13:20:38 <sdague> that's varchar(255)
13:20:55 <alex_xu> name is regex matching, it can't get benefit from db index
13:20:59 <johnthetubaguy> yeah, I wasn't every clear, it sounds like that might already by not worth an index, but we need to check that
13:21:06 <sdague> right, the regex issue is a whole other issue
13:21:24 <johnthetubaguy> yeah, lets table that discussion for now
13:21:40 <sdague> especially because that's not going to be supportable with elastic search (searching there is different)
13:21:51 <johnthetubaguy> +1
13:21:53 <sdague> ok, so I think on #3
13:21:56 <johnthetubaguy> yeah
13:22:06 <sdague> my feeling is most of the admin vs. non-admin is artificial
13:22:19 <sdague> excepting nodenames
13:22:35 <sdague> so, project_id, no reason to exclude that for non admin
13:22:41 <sdague> because they are already scoped
13:22:54 <johnthetubaguy> true, it just returns zero results I guess
13:23:10 <sdague> and... in the future of hierarchical multi tenancy, it actually will mean something for non cloud admin
13:23:22 <johnthetubaguy> yep, thats true
13:23:30 <sdague> so just start with the basic assumption that we try to make everything the same between the 2 groups
13:23:48 <sdague> the only place this can't be is when things start leaking cloud topology
13:23:57 <sdague> which is the node/host fields
13:24:17 <alex_xu> yea
13:24:17 <sdague> so, short term, that's the only admin adds
13:24:20 <johnthetubaguy> yeah, no breaking out of token scope, and no topology leak, agreed with that
13:24:45 <sdague> longer term, it would honestly be nice if those fields did expose to normal users, in the hashed format
13:24:58 <sdague> and could be filtered sorted
13:25:11 * johnthetubaguy forgot thats not upstream already... I guess
13:25:33 <alex_xu> we have that in the api response
13:25:39 <sdague> so the difference isn't the filter/query side, it's just in the representation used, whether or not we are hiding topology
13:25:42 <alex_xu> but calculated in the python, so can't filter and sort
13:25:53 <sdague> alex_xu: right, so it's not an Ocata thing
13:26:07 <sdague> but, something we should figure out if there is a way forward on in Pike+
13:26:26 <alex_xu> yea
13:26:26 <johnthetubaguy> yeah, its hashed on tenant_id, so I guess which tenant_id will be used for each instance
13:26:36 <sdague> alex_xu: because, we totally could calculate it in the db
13:26:50 <alex_xu> sdague: got it
13:26:50 <johnthetubaguy> yeah, thats what I was thinking
13:26:55 <johnthetubaguy> anyways, keeping moving
13:26:56 <sdague> so...
13:27:16 <sdague> I'm hoping this means we've agreed to the following:
13:27:31 <sdague> 1) we should have a strict whitelist
13:27:49 <sdague> 2) it's ok to defer the issue that everything in the whitelist has an index
13:28:25 <sdague> 3) the admin list differences should only be node/host - and that's a temporary stop gap until we have the better story there
13:28:33 <alex_xu> I feel we can't have everthing have an index. anyway that is second question we will discuss
13:28:46 <sdague> alex_xu: I agree with you
13:29:26 <alex_xu> sdague: ok, now i feel same page with you  :)
13:29:36 <johnthetubaguy> I am +1 for all three
13:29:55 <johnthetubaguy> still getting my head around (3), I like its simplicity
13:30:32 <sdague> alex_xu: I think we probably want to write up some doc about performance tuning filters, knowing that we're going to do our best here, but that site specific workloads might be using filters we didn't see as high priority
13:31:13 <johnthetubaguy> I was only trying policy instead of (2) as a compromise with those against (2), but its great to avoid that
13:31:16 <alex_xu> sdague: ah, that is good idea
13:31:29 <alex_xu> and we should suggestion user don't user all_tenant=1
13:31:47 <sdague> I also think that searchlight being needed for cells v2 ... a bunch of the performance characteristics are going to change
13:31:51 <johnthetubaguy> alex_xu: so the token scope wins, so its meaningless I think
13:31:57 <sdague> johnthetubaguy: right, that
13:32:02 <johnthetubaguy> sdague: +1 that blows the old rules out the water
13:32:22 <alex_xu> johnthetubaguy: sorry, I didn't get that
13:32:28 <sdague> johnthetubaguy: which is why I'm fine on defering #2, because it's going to change again, so not make guaruntees
13:33:01 <sdague> alex_xu: we should be doing the token scoping of queries such that all_tenant means "all tenants I can see"
13:33:02 <johnthetubaguy> sdague: yeah
13:33:15 <sdague> if role=admin, that's everything
13:33:21 <sdague> if not, that's a smaller thing
13:33:27 <sdague> which is just your own
13:33:32 <johnthetubaguy> yeah, I like that view
13:33:51 <sdague> in hierarchical multi tenancy it would be from your part of the tree down
13:34:14 <johnthetubaguy> right now restricting to "what I can see" adds the where clause that hits the index we want to hit
13:34:35 * alex_xu probably understand
13:35:36 <sdague> think of it like: `ps` vs `ps a`
13:35:52 <sdague> sorry, `ps` vs `ps x`
13:36:04 <sdague> by default, show me mine
13:36:12 <sdague> but I can ask to see everything I'm allowed
13:36:28 <sdague> under container systems, ps x is returning very small amount of stuff
13:36:42 <sdague> because you aren't allowed the whole system
13:37:29 <alex_xu> emm...ok
13:38:19 <sdague> alex_xu: did I make it less clear? :)
13:38:51 * johnthetubaguy things he should stop doing "sudo ps ax"
13:39:16 <gmann> :)
13:39:17 <sdague> heh
13:39:22 <alex_xu> sdague: I guess, you are looking for hierarchical multi tenancy, that is more than all_tenants
13:39:25 <Kevin_Zheng> ：）
13:39:30 * johnthetubaguy makes hammer bashing noises
13:39:41 <alex_xu> :)
13:39:57 <alex_xu> never mind, maybe I just need to read the meeting log again :)
13:40:37 <johnthetubaguy> sdague: thanks for that, loving (2) and (3), I think thats a good re-starting point
13:40:42 <sdague> alex_xu: yeh, I'm just saying we clarify the meaning of all_tenants now to one that is compatible with future multi tenancy
13:41:10 <alex_xu> sdague: oh yeah, i think i understand
13:41:40 <alex_xu> so we can forget the policy now?
13:42:01 <sdague> alex_xu: I hope so
13:42:06 <sdague> johnthetubaguy: ^^^ ?
13:42:24 <alex_xu> ok, that point didn't resolve yet :)
13:42:35 <johnthetubaguy> I like your breakdown of the issue basically
13:42:40 * alex_xu is still in lost
13:42:54 <sdague> ok, I thought that with 1), as long as we keep that whitelist large enough, there is no need for the policy
13:43:20 <sdague> the issue with the policy was if we restricted the list further in 2) to only match indexes, we removed things people needed
13:43:29 <johnthetubaguy> its (2) that stops the policy for me, I always wanted (1), but either way
13:43:54 <sdague> if we are ok leaving things in the whitelist that don't have indexes, we do not need the policy point
13:44:09 <johnthetubaguy> yeah
13:44:31 <sdague> and I think that's a better trade off. Some queries are slower than they could be, but we aren't making the behavior site specific.
13:44:52 <sdague> knowing in one or two cycles the performance of searches is going to change dramatically anyway
13:45:31 <johnthetubaguy> yeah, I am not too fussed with the performance, as for non-admins I don't think its ever going to break too badly
13:46:24 <johnthetubaguy> I was trying to compromise with the folks that cared about the performance, but it left us in a worse position
13:46:57 <johnthetubaguy> for the folks super worried about performance, I think this can be accepted as a step in the right direction
13:47:02 <sdague> yeh
13:47:19 <sdague> it also gives us an idea of what we'd want to ask out of optimizing in searchlight
13:47:27 <johnthetubaguy> yeah, very true
13:47:29 <sdague> as we'll say "here are the fields we let people filter on"
13:47:39 <sdague> what can we do with those?
13:48:31 <sdague> ok... so I guess, what are the actions to move this forward?
13:49:01 <alex_xu> yea...I just want to ask that. I feel i'm still in lost status.
13:49:02 <johnthetubaguy> update the spec, update the prototypes?
13:49:52 <alex_xu> so....for now, what I should keep in the whitelist?
13:50:11 <sdague> ok, so how about this:
13:50:12 <johnthetubaguy> everything thats in the API response, and works today, I guess?
13:50:23 <alex_xu> ok
13:50:34 <sdague> 1) johnthetubaguy verify the whitelist looks sane to him (he has the most interaction with ops here)
13:50:53 <sdague> should we take the summary out to the ML for context?
13:51:10 <alex_xu> that sounds better
13:51:23 <sdague> because the spec is getting sliced and diced and sometimes I think the forest is lost for the trees
13:51:25 <Kevin_Zheng> yeah
13:51:32 * gmann need to read all review comments also. ll be nice to learn lot of things.
13:51:43 <sdague> 2) sdague to write summary of current high level agreement
13:51:47 <sdague> to ML
13:51:55 <johnthetubaguy> I am good with the ML, it might be a good point to start a new spec honestly
13:52:13 <johnthetubaguy> should be the same change-id, but start over might help here
13:52:24 <sdague> johnthetubaguy: yeh, well, we're so late at this point, I think we need to figure out how the spec lands ASAP
13:53:06 <johnthetubaguy> sdague: true
13:53:28 <sdague> alex_xu / Kevin_Zheng can we start coding this in parallel assuming this will be the agreement?
13:53:47 <Kevin_Zheng> sure
13:53:57 <alex_xu> sdague: yea, after I figure out the plan from your summary :)
13:54:00 <johnthetubaguy> we have the validation framework already up I guess?
13:54:16 <alex_xu> #link https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/add-whitelist-for-server-list-filter-sort-parameters
13:54:18 <sdague> alex_xu: ok, well I'll make breakfast first, then write up that email :)
13:54:18 <johnthetubaguy> well, just is all about tweaking the whitelist, it can start from the old one I guess
13:54:23 <Kevin_Zheng> I will keep sync with alex
13:54:23 <alex_xu> ^ yea the top two patches
13:54:38 <sdague> johnthetubaguy: yeh, honestly, I'm good with whatever you sign off on for the whitelist
13:54:40 <alex_xu> sdague: thanks :)
13:54:51 <sdague> we'll make you the required ACK for that
13:55:06 <johnthetubaguy> OK
13:55:25 <sdague> and we'll try to get this all sorted by the end of the week?
13:55:34 <sdague> A week from now is my last day in the office for the year
13:55:37 <johnthetubaguy> I think we have to, thats a good goal
13:55:43 <johnthetubaguy> yeah, same
13:55:56 <johnthetubaguy> actually, its a week yesterday for me
13:56:30 <alex_xu> so will we have meeting next week?
13:58:07 <johnthetubaguy> maybe? lets keep in close contact over the week though
13:58:16 <alex_xu> ok, cool
13:58:26 <alex_xu> 2 mins left
13:58:55 <alex_xu> who is updating the spec?
14:00:13 <alex_xu> ok... it time close the meeting
14:00:14 <alex_xu> thanks all
14:00:16 <alex_xu> #endmeeting