13:01:01 <alex_xu> #startmeeting nova api 13:01:02 <openstack> Minutes (text): http://eavesdrop.openstack.org/meetings/nova_api_startmeeting_nova_api/2017/nova_api_startmeeting_nova_api.2017-03-22-13.00.txt 13:01:03 <openstack> Log: http://eavesdrop.openstack.org/meetings/nova_api_startmeeting_nova_api/2017/nova_api_startmeeting_nova_api.2017-03-22-13.00.log.html 13:01:04 <openstack> Meeting started Wed Mar 22 13:01:01 2017 UTC and is due to finish in 60 minutes. The chair is alex_xu. Information about MeetBot at http://wiki.debian.org/MeetBot. 13:01:05 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 13:01:07 <openstack> The meeting name has been set to 'nova_api' 13:01:14 <mriedem> o/ 13:01:22 <gmann> o/ 13:01:23 <cdent> mriedem: are you everywhere now because of your working-from-home-ness? 13:01:26 <johnthetubaguy> o/ 13:01:31 <mriedem> cdent: yes 13:02:02 <alex_xu> mriedem: you worked at office before, i thought no-one will work at office in the US 13:02:16 <mriedem> alex_xu: the local ibm people do 13:02:17 <alex_xu> #topic priorities 13:02:36 <alex_xu> #link https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/policy-docs 13:02:51 <alex_xu> the spec for policy docs merged, and already have a lot of patches ready for review \o/ 13:03:33 <gmann> want to ask how to show policy for deprecated APIs - #link https://review.openstack.org/#/c/448267/ 13:03:39 <gmann> as test line ok? 13:03:42 <Kevin_Zheng> o/ 13:03:48 <gmann> text 13:03:56 <johnthetubaguy> so deprecated APIs are still APIs 13:04:06 <johnthetubaguy> thats different to deprecated policy rules that are about to die 13:04:12 <gmann> yea 13:04:16 <alex_xu> yea 13:04:40 <alex_xu> we only to note deprecated API in the api-ref. 13:04:50 <alex_xu> s/only/only need/ 13:05:12 <johnthetubaguy> I think we should get the basics in there, and see how we look once thats done 13:05:42 <johnthetubaguy> its a shame to bloat loads saying that the API is deprecated, if we don't need to. 13:05:42 <alex_xu> yea 13:06:01 <johnthetubaguy> #link https://docs.openstack.org/developer/nova/sample_policy.html 13:06:08 <johnthetubaguy> its starting to look much better already ^ 13:06:48 <mriedem> yeah it is 13:07:07 <gmann> yea, so we do show note about deprecated API like volume API in that sample. 13:07:28 <johnthetubaguy> I thought we said about we probably don't need to mention it 13:08:18 <johnthetubaguy> we might deprecated a policy rule, but thats different 13:08:31 <gmann> yea that is separate. 13:08:31 <alex_xu> johnthetubaguy: do we need a space line between the doc and rule? a little hard to figure which one is rule. 13:09:16 <johnthetubaguy> alex_xu: sure, thats an oslo.policy generator change though 13:09:51 <johnthetubaguy> sneti or aunnam might fancy fixing that up ^ 13:10:10 <alex_xu> johnthetubaguy: ah yes, that true. I thought we can add a '\n', but you are right, that should be done by oslo.policy 13:10:12 <johnthetubaguy> oh I quick note on "os_compute_api:os-attach-interfaces:discoverable" and the like 13:10:45 <johnthetubaguy> I asked them not to document those, because I think we can remove those 13:11:20 <gmann> +1. 13:11:26 <alex_xu> yea, hope we can remove those. really needn't waiting for one more release 13:11:41 <gmann> patrole project want to test discover policy which they should not right? 13:11:59 <johnthetubaguy> that in the follow up spec, removing those 13:12:11 <johnthetubaguy> gmann: correct, its not worth it 13:12:22 <gmann> only thing we did document that those are going to remove and they thought its valid policy 13:12:26 <alex_xu> johnthetubaguy: or just create a separated bp for it, then people can begin to kill those directly? 13:12:27 <johnthetubaguy> gmann: I still need to work out what that thing does at some point, probably before we approve the next spec 13:12:36 <johnthetubaguy> alex_xu: can do 13:12:38 <gmann> +1, ll convey that 13:13:35 <johnthetubaguy> gmann: I hadn't spotted they still existed till we started this, there are quite a few rules sneti and aunnam have found that actually are unused 13:14:05 <johnthetubaguy> I am sure, just like with the config, there is a bunch of stuff that will need tidying up 13:14:13 <gmann> yea 13:14:36 <gmann> i feel its good to have patrole gate job on nova side ? 13:14:41 <johnthetubaguy> anyways, not sure there is much else I wanted to discuss on that one (I am keen to talk about the other two specs I guess) 13:15:22 <alex_xu> johnthetubaguy: would OSIC guys want to create a bp for removing discovery entry? 13:15:49 <johnthetubaguy> alex_xu: its already on their TODO list, I can ask them to create a separate specless BP for that 13:16:02 <alex_xu> johnthetubaguy: thanks 13:16:20 <johnthetubaguy> gmann: I think we need to work out where it fits, I was really wanting in tree functional tests for the policy stuff 13:16:38 <alex_xu> thanks OSIC guys, thanks sneti and aunnam 13:16:58 <johnthetubaguy> +1 great to see us making progress on this stuff 13:17:14 <alex_xu> #link https://review.openstack.org/#/c/433037/ 13:17:17 <gmann> johnthetubaguy: yea functional tests always nice. i was thinking we can use patrole effort also for more testing 13:17:30 <alex_xu> johnthetubaguy: ^ you want to talk that more 13:17:32 <gmann> but not sure if sometime that can delay the changes etc 13:18:04 <johnthetubaguy> so the spec 13:18:14 <johnthetubaguy> really I am wondering whats needed to get that approved / agreed 13:18:39 <johnthetubaguy> had some great comments from alex_xu sneti and aunnam to clean up the text in there 13:19:08 <johnthetubaguy> gmann: having both might be correct, I just don't know right now 13:19:23 <johnthetubaguy> I should copy the summary I guess 13:19:36 <gmann> sure 13:19:39 <johnthetubaguy> Here we separate the concerns of what resources user has access to and 13:19:39 <johnthetubaguy> what actions the user is allowed to do. 13:19:44 <johnthetubaguy> thats basically it 13:19:57 <johnthetubaguy> remove scope checks from policy.json 13:20:58 <alex_xu> yea, mix the role check and scope checks into one rule, that makes people hard to understand 13:21:16 <johnthetubaguy> also makes it easy to break Nova's API in the policy file 13:21:37 <johnthetubaguy> now we are not breaking the whole user_id thing just yet, while we get hierarchical quotas implemented 13:21:40 <alex_xu> and scope check isn't something people need to change 13:22:00 <johnthetubaguy> alex_xu: +1 thats the hope, well its something we don't want them to change 13:22:18 <johnthetubaguy> doing that makes this one much easier 13:22:20 <johnthetubaguy> #link https://review.openstack.org/#/c/427872 13:22:43 <johnthetubaguy> basically having some proper default roles for various use cases, beyond user vs global admin 13:23:21 <johnthetubaguy> mriedem: do you think you will get chance to hit those two remaining policy specs? 13:23:51 <mriedem> johnthetubaguy: umm 13:23:54 <mriedem> today? 13:23:57 <alex_xu> I didn't read that spec again. the orignal one feel mix too much thing: more role, deprecated policy..etc 13:24:15 <johnthetubaguy> mriedem: sometime this week would be cool, not worried about it being today 13:24:40 <mriedem> honestly i didn't grok those very well the last time i looked 13:24:44 <johnthetubaguy> alex_xu: yeah, the key bit is the new role, the deprecated policy is needed to implement that 13:24:44 <mriedem> around the time of the ptg 13:24:53 <johnthetubaguy> mriedem: that first one reads much better now I hope 13:25:11 <mriedem> ok i guess if i still don't understand the problem i'll yell about it 13:25:17 <johnthetubaguy> mriedem: totally 13:25:20 <mriedem> i think it was missing some clear examples before 13:25:20 <alex_xu> johnthetubaguy: ok, i will try to read that again 13:26:14 <alex_xu> #link https://review.openstack.org/435485 13:26:25 <alex_xu> mriedem: johnthetubaguy also have a poc ^ 13:26:42 <johnthetubaguy> yeah, its just for that first one 13:27:20 <johnthetubaguy> gmann: thats the functional tests I was on about earlier ^ 13:27:47 <johnthetubaguy> now there are some fun details here, between 404 errors and 403 errors 13:27:56 <gmann> johnthetubaguy: i see, ll look into those tomorrow. 13:28:08 <johnthetubaguy> DB checks triggering 404s and policy triggering (sometimes) 403 errors 13:29:04 <johnthetubaguy> but the basic idea is to have zero API impact, its just there are some inconsistencies that need to be ironed out here and there 13:29:55 <alex_xu> for the policy check we return 403, for scope check we return 400, i thought 13:30:19 <johnthetubaguy> well, its more complicated I think 13:30:20 <gmann> 404 on policy failure ? 13:30:41 <alex_xu> johnthetubaguy: the 404 and 403 both are expected return code, so i think it's fine. we won't impact the API 13:30:49 <johnthetubaguy> alex_xu: agreed 13:31:00 <johnthetubaguy> my worry is information disclosure 13:31:11 <johnthetubaguy> someone with no access to a server, gets a 404 on that URL today 13:31:22 <johnthetubaguy> so you can't tell if you guessed the instance uuid correctly 13:31:27 <johnthetubaguy> (for example) 13:31:41 <johnthetubaguy> I vote we keep that behaviour the same, even when the check is no longer in the DB code 13:32:19 <cdent> guessing uuids is a new olympic sport 13:32:27 <cdent> but yeah, it is a tricky issue 13:32:47 <alex_xu> johnthetubaguy: you already separate the scope check out of policy rule. that means the policy rule is only about whether you have permission to access the API 13:32:49 <gmann> johnthetubaguy: you mean 404 on scope check right so people would not know anything about internal restriction etc 13:33:00 <alex_xu> so we return 403 for polciy check is right at here. The API endpoint existed, you are just not allowed to access 13:33:40 <alex_xu> for scope check, for a uuid, we will return 404. you can't see that instance. and we didn't leak something 13:34:14 <johnthetubaguy> yeah, something like that 13:34:48 <johnthetubaguy> if you got 403 for all valid and invalid uuids, that would be fine, for example 13:35:05 <johnthetubaguy> its the difference in return value that means you can probe for information 13:35:32 <johnthetubaguy> anyways, thats all implementation detail I think, because it exact circumstances are different for different APIs 13:35:49 <johnthetubaguy> the functional tests we create will help discover these things 13:36:01 <johnthetubaguy> (at least, thats how I found out there was a problem, when I wrote the POC) 13:39:14 <alex_xu> i need check the spec about the functional tests 13:39:33 <johnthetubaguy> I think I said tests that pass before and after the change. 13:39:46 <gmann> from first look those are really useful and ll find many issue as johnthetubaguy mentioned 13:39:51 <johnthetubaguy> i.e. we move the check from the DB into the new logic, and the same tests should check its all good 13:40:02 <gmann> +1 13:40:16 <johnthetubaguy> gmann: it shocked me how interesting they will be :) 13:40:51 <alex_xu> ok, anything more about policy? 13:41:12 <gmann> johnthetubaguy: but we need to complete all those tests before new policy way 13:41:51 <gmann> i think those will be huge with all negative combination for all current policy 13:42:01 <johnthetubaguy> well, need to add the test in each code path, before converting that code path 13:42:20 <johnthetubaguy> we certainly shouldn't add them all first 13:42:35 <gmann> yea that's much feasible 13:43:03 <johnthetubaguy> most of that test I added is common boilerplate stuff, I think 13:43:18 <johnthetubaguy> the per API stuff is quite minimal I hope 13:43:29 <johnthetubaguy> but time will tell 13:43:43 <johnthetubaguy> anyways, feedback very welcome on all that stuff 13:44:00 <johnthetubaguy> I would love for us to make some progress on at least the first of those two specs this cycle 13:44:12 <alex_xu> +1 13:44:17 <gmann> \o/ 13:44:18 <johnthetubaguy> granted, the third spec should not be started until the first two are completed 13:44:41 <alex_xu> yea 13:45:04 <johnthetubaguy> cool, seems like we are roughly agreed on the approach, which is all good 13:45:26 <alex_xu> yes 13:46:20 <alex_xu> I didn't get progress for the poc of remove stevedore, hope I bring news in next week 13:46:52 <alex_xu> we have three items in the open, if nothing more for priorities, let us jump to open 13:46:52 <cdent> alex_xu++ on that stuff. 13:46:58 <alex_xu> cdent: thanks 13:47:00 <cdent> (the removal of stevedore) 13:47:16 <johnthetubaguy> yeah, that tottally needs removing 13:47:25 <alex_xu> #topic open 13:47:31 <alex_xu> mriedem: your turn 13:47:34 <mriedem> yay 13:47:43 <mriedem> i just replied to comments in https://review.openstack.org/#/c/447149/ 13:47:53 <johnthetubaguy> (FWIW, we hit some strangeness around limits and used limits looking at that policy stuff, so the collapse of those would be nice at some point) 13:47:53 <mriedem> alex_xu: some clarifying questions for you 13:47:59 <alex_xu> mriedem: thanks 13:48:05 <mriedem> ken'ichi also brought up the os-hosts API 13:48:08 <mriedem> which i didn't even think of, 13:48:23 <mriedem> but now that i look at it, is basically the same as the os-services API, except it has some additional actions, like stop/start/reboot 13:48:30 <mriedem> so that's pretty ugly 13:48:40 <johnthetubaguy> oh, most of those are broken too 13:48:42 <mriedem> i don't know why we have 2 APIs for the same resource, but i've questioned this over my tenure in nova 13:48:59 <mriedem> johnthetubaguy: i haven't looked at what supports those APIs, but assume it's xen 13:49:08 <johnthetubaguy> mriedem: quite the opposite I think 13:49:09 <mriedem> because xen is always the oddball in the API 13:49:10 <mriedem> oh 13:49:34 <johnthetubaguy> I think its more about clustered hypervisors where that distinction becomes important 13:49:52 <johnthetubaguy> quite how you start a host that is shutdown and no nova-compute service running on it, for example 13:50:11 <johnthetubaguy> it might be left over from the old pre-ironic stuff, I haven't dug to check that 13:50:33 <alex_xu> i feel we add new 'uuid' field explicitly better than change the id field's value to a uuid 13:50:53 <johnthetubaguy> mriedem: so I guess we need to stage this work somehow, we could go with the more obvious fixes first 13:51:09 <mriedem> looks like xen and hyperv support host_power_action 13:51:11 <mriedem> but vcenter doesn't 13:51:15 <mriedem> nor libvirt or ironic 13:51:34 <johnthetubaguy> mriedem: hmm, must have been baremetal than, I know its mostly broken for xen 13:51:52 <johnthetubaguy> probably worth deprecating that API I guess 13:51:55 <mriedem> maybe first step is opening a bug about actually documenting wtf os-hosts does and what supports it :) 13:52:04 <johnthetubaguy> yeah 13:52:09 <johnthetubaguy> then deprecating it 13:52:19 <mriedem> ok so i think i basicaly have two open questions for the spec, 13:52:47 <mriedem> 1. do we change the os-services action APIs to take a service_id rather than rely on looking up the service by hostname/binary in the body? 13:52:57 <mriedem> e.g. PUT /os-services/{service_id}/disable 13:53:16 <mriedem> PUT /os-services/disable-log-reason 13:53:17 <mriedem> { 13:53:17 <mriedem> 'disabled_reason': 'host os upgrade maintenance' 13:53:17 <mriedem> } 13:53:34 <johnthetubaguy> I am +1 that myself 13:53:36 <mriedem> err PUT /os-services/{service_id}/disable-log-reason 13:53:41 <johnthetubaguy> yeah, that 13:53:59 <mriedem> alex_xu: gmann: cdent: opinions on that? 13:54:06 * cdent is thinking 13:54:21 <mriedem> feel free to just chime in on the spec when you have time 13:54:23 <alex_xu> so we want to introduce that new action way? 13:54:28 <mriedem> i want dansmith to review it before i make changes 13:54:36 <alex_xu> not /os-services/{service_id}/action, anymore? 13:54:54 <dansmith> ack 13:55:04 <cdent> i'll give the review a real review later today 13:55:04 <mriedem> alex_xu: we already have PUT /os-services/disable today 13:55:11 <gmann> and user will get service_id from list 13:55:13 <mriedem> with a body like: 13:55:13 <mriedem> { "host": "host1", "binary": "nova-compute" } 13:55:26 <alex_xu> mriedem: at least we consistent with other existed apis i think 13:55:30 <mriedem> so that becomes just PUT /os-services/{service_id}/disable 13:55:32 <mriedem> with no body 13:55:45 * edleafe wakes up. Long night 13:55:45 <mriedem> because we don't need the host and binary to identify the service if we have the id 13:56:00 <cdent> in general using identifiers in the URL is good 13:56:15 * alex_xu reminds that we only left 4 mins 13:56:16 <gmann> IMO /disable explicitly much cleaner than /action 13:56:18 <mriedem> yeah honestly i had assumed this was how things already worked in here 13:56:22 <johnthetubaguy> cdent: is that the correct URL for an action, rather than our silly /action catch all thing? 13:56:24 <gmann> yea +1 on those 13:56:27 <mriedem> ken'ichi didn't want /action 13:56:29 <mriedem> which i'm fine with 13:56:53 <cdent> johnthetubaguy: unresolved huge debate that was so huge people gave up 13:56:55 <johnthetubaguy> yeah, I think avoiding the catch all /action is probably good, I can't remember if there is already an API WG thingy for this 13:57:01 <johnthetubaguy> cdent: ah, OK 13:57:02 <cdent> it was abandoned 13:57:09 <mriedem> the other thing, 13:57:18 <cdent> because it looked like soap 13:57:21 <edleafe> johnthetubaguy: cdent: usually POST is more forgiving, no? 13:57:23 <mriedem> 2. do we also want to do this for os-hosts if we're doing it for os-services (since they are basically the same api)? 13:57:26 <mriedem> i'd think yes 13:57:38 <mriedem> "yes" to also doing os-hosts 13:57:42 <johnthetubaguy> well, the option is to just deprecate os-hosts 13:57:50 <gmann> me also not comfirtable for /action 13:58:01 <johnthetubaguy> and roll any remaining things of interest into os-services 13:58:01 <mriedem> johnthetubaguy: if we deprecate os-hosts, 13:58:09 <mriedem> then we have to move the stop/start/reboot actions from os-hosts to os-services 13:58:17 <mriedem> which we could do 13:58:22 <johnthetubaguy> I think we could argue they shouldn't live in our API 13:58:25 <mriedem> it just starts to become a hell of a big spec 13:58:25 <johnthetubaguy> so there is no replacement 13:59:06 <mriedem> with 1 minute left, move discussion to the spec? 13:59:15 <cdent> yes 13:59:16 <mriedem> i also wanted sdague's thoughts on all of this too 13:59:16 <alex_xu> yea 13:59:21 <gmann> yes 13:59:28 <mriedem> anyway, thanks 13:59:32 <alex_xu> edleafe: sorry, we didn't have enough time for your item 13:59:44 <edleafe> np 13:59:51 <edleafe> I slept in anyway :) 13:59:58 <johnthetubaguy> +1 on wanting sdague's view on this 14:00:02 <alex_xu> ok, it's time to close the meeting, thanks all, let us back to the nova channel 14:00:04 <alex_xu> #endmeeting