13:00:04 #startmeeting nova api 13:00:05 Meeting started Wed Aug 3 13:00:04 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:06 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 13:00:08 The meeting name has been set to 'nova_api' 13:00:11 who is here today? 13:00:14 o/ 13:00:40 hi 13:00:52 hi 13:01:14 good morning and evening 13:01:27 \o 13:01:36 hi 13:01:39 alex_xu: good UGT morning! 13:01:58 edleafe: ah, so smart! 13:02:11 o/ 13:02:24 so, let us start the meeting 13:02:32 #topic action from previous meeting 13:02:43 * alex_xu gmann_ to start on user_id policy patches 13:03:00 gmann_: do you have update for this? 13:03:03 alex_xu: yea, i started but not pushed patch for that 13:03:21 alex_xu: may be this week i will start adding those 13:03:22 gmann_: do you need help, I guess that need a lot of patches? 13:03:38 gmann_: cool! 13:03:48 alex_xu: yea, i will ping you tomorrow. help is always great :) 13:04:36 gmann_: cool, and Kevin_Zheng ask me about anything help need in api team, so I and Kevin_Zheng both ready for help you 13:04:57 alex_xu: cool. Thanks. i will let you guys know. 13:05:08 gmann_: np, cool 13:05:08 then may be we can out all these soon. 13:05:12 feels like we should get a patch up to do one action, then get others to follow the pattern? 13:05:13 #link https://review.openstack.org/324068 13:05:22 the spec still didn't get merge yet 13:05:23 alex_xu: but spec still need +A 13:05:29 johnthetubaguy: yea 13:05:30 yea 13:05:40 johnthetubaguy: +1 13:05:46 sdague: I have one question, do we move target parameter for other actions 13:05:54 s/move/remove 13:06:22 alex_xu: are they already there for other actions? 13:06:31 I thought the issue was they weren't anywhere 13:06:53 sdague: yea, I think there are few, like start action have target parameter 13:07:10 well, right now, let's just worry about the additions 13:07:16 but I didn't check the full list, if we want to remove that, I will scan all the api 13:07:23 ok 13:07:23 * mriedem joins late 13:07:27 and we can evaluate removes later 13:07:30 honestly, right now, the tenant_id check is basically a no op in the policy layer 13:07:38 mriedem: there is a spec for you to review here: https://review.openstack.org/324068 13:07:50 yup, will do this morning 13:07:59 * alex_xu just +1 for the spec 13:08:25 johnthetubaguy: right, that's true, with policy in code we can probably handle that better. There was actually a bug about that recently I triaged. 13:09:04 * alex_xu needs check what happened 13:09:28 so we are cool at here 13:09:46 * alex_xu please review https://review.openstack.org/#/q/topic:bp/discoverable-policy-cli,n,z 13:10:06 emm...I didn't do that, but I will try to reach that asap 13:10:31 there still have one patch mark as WIP 13:11:22 I guess no comment at here 13:11:27 sdague: interesting, I thought we were OK as we check when we fetch the instance form the DB, but thats basically hard coded right now 13:11:41 johnthetubaguy: right, that was actually the bug 13:11:59 * alex_xu speak too fast... 13:12:04 the desire to put a manager project policy in place 13:12:18 it was from sam at nectar, what's his last name again? 13:12:23 morrison 13:12:55 #link https://bugs.launchpad.net/nova/+bug/1607602 13:12:55 Launchpad bug 1607602 in OpenStack Compute (nova) "non admin project policy.json declarations ignored for most instance actions" [Medium,Confirmed] 13:13:13 thanks, that's what I needed, I really wish launchpad had "bugs I've commented on" feature 13:13:20 or bugs I changed something with 13:13:21 sdague: ah, gotcha 13:13:25 sdague: +1 to that 13:14:17 johnthetubaguy: so it's related in that we still have some admin context hard coding at the db layer which we need to eventually remove, and with policy in code it should be safer to do so, as we'll have sane backstops 13:14:34 anyway, it's O at least for that, but it was related 13:14:37 we can move on I think 13:14:54 ok 13:15:18 I need check the bug later, still didn't get what happen... 13:15:47 #topic API Priorities 13:16:21 So i think we need discuss the deprecation API 13:16:23 #link https://review.openstack.org/350277 13:17:16 I have one idea, we should keep os-interface and deprecate the os-virtual-interface 13:17:16 yeah, so, that's the spec based on the ML thread 13:17:40 i don't really like that idea if we plan on exposing vif tags out of the api at some point 13:17:54 and exposing vif tags is the only reason i'd keep os-virtual-interfaces around 13:18:11 mriedem: we can expose that on os-interface in next release 13:18:33 mriedem: the problem for os-virtual-interfaces is only work for instance which created after Newton 13:18:46 alex_xu: not if we use the instance.info_cache 13:19:24 the vif tagging was added pretty recently right, what's the original justification for putting that in the nova API at all? 13:19:26 mriedem: emm...yea, we can 13:20:01 sdague: vif tagging was 2.32 13:20:15 you pass the tags for ports/bdms when creating the server 13:20:28 i think artom/dansmith plan for that to be in os-attach-interface at some point also 13:20:46 yeah, that should match boot, and you should be able to query what you did 13:20:51 mriedem: we can do that, and fix it as bug. Another problem is we still keep two endpoint os-interface and os-virtual-interface, that is confuse 13:21:11 ok, is this not returned in the normal server record? 13:21:18 sdague: no 13:21:23 why? 13:21:26 it feels like it lives in there 13:21:37 I do get the fact that we should be able to view what we did 13:21:53 but if this was added so that it was a param on boot, it seems like it should be in the server record 13:22:16 anyone got a link to the spec for this? 13:22:26 https://review.openstack.org/#/c/350277/ 13:22:42 mriedem: no the vif tagging one 13:22:58 http://specs.openstack.org/openstack/nova-specs/specs/newton/approved/virt-device-role-tagging.html 13:23:51 ah, right, you need to be able to manage the tags 13:24:19 so this was landed in neutron in some way as well? 13:25:15 no 13:25:17 purely nova 13:25:25 and this was done only for boot. not for volume-attach nd interface-attach 13:25:29 the tag is stored in the bdm and virtual_interface tables 13:25:29 so nova is the only keeper of these tags 13:25:36 yes 13:25:51 ok, so os-virtual-interface is the only management API for those? 13:26:06 today os-virtual-interface is the lookup for virtual_interfaces, yes 13:26:13 which only works for nova-net right now 13:26:13 what manages those 13:26:23 do we need manage that? 13:26:29 nova-net and neutron both populate virtual_interfaces now 13:26:38 as of 2.32 13:26:57 thinking of whether those tag only used by instance startup 13:27:01 looking back it probably would have made more sense for cinder and neutron to have tags on their resources 13:27:16 but you have things like ephemeral and swap bdms in nova 13:27:19 that can be tagged 13:27:39 but we probably should have tagged ports in neutron 13:27:52 oh but then it wouldn't work for nova-net 13:28:03 which, this spec has been around since before nova-net was deprecated 13:28:09 so that's probably why that wasn't taken into consideration 13:28:32 I mean you can argue both ways, one port might have a different tag when attached to a different instance, so maybe its correct 13:28:50 johnthetubaguy: true, the vifs are scoped to an instance and a port 13:29:01 so... here is the thing. It's not super clear to me that either of these API resources are strictly proxies that we don't need 13:29:16 which makes this complicated enough, that I don't want to rush it 13:29:25 what do you mean by api resources? 13:29:33 os-interface and os-virtual-interfaces? 13:29:36 yeh 13:29:36 or bdms and ports? 13:29:37 ok 13:29:51 os-interface GET methods are pure proxy to neutron 13:29:58 to list and show ports 13:30:00 only works for neutron 13:30:10 os-virtual-interface has a single GET method which only works for nova-net 13:30:17 is it instance specific in os-interface? 13:30:23 but that's just an impl detail right now, we can make it work for neutron in ocata 13:30:29 johnthetubaguy: yes 13:30:31 johnthetubaguy: yes 13:30:34 scoped to a server in both apis 13:30:48 right, and untangling this into something coherent feels like it's going to take time and concentration 13:30:50 the show method in os-interface is goofy though, 13:31:06 you pass the instance to show a port, but we don't actually use the instance, we just check that it exists and is bound to the port 13:31:33 I think we should just punt on this until Ocata 13:31:37 i feel like i've spent several days and plenty of concentration on this since last week 13:31:44 mriedem: right, agreed 13:31:49 I am +1 the punt, it needs thought 13:31:53 +1 13:31:53 and it still isn't super clear to folks yet 13:32:01 the create interface seems important 13:32:10 johnthetubaguy: create/delete don't change 13:32:14 only the GET methods 13:32:17 and the list seems useful, the others less so 13:32:26 list ports? 13:32:29 you can do that in neutron 13:32:30 so its odd having a half baked resource 13:32:40 mriedem: sure but this, what ports does my instance have 13:32:46 so, we can make the same argument for listing floating IPs for a server 13:32:50 thats a good nova question 13:32:55 johnthetubaguy: ^ 13:33:01 mriedem: nope, thats a property of a port, thats neutron bussiness 13:33:07 we kind of need to shit or get off the pot with these proxies 13:33:12 the port has the device_id set 13:33:17 but anyways, we seem agreed this needs careful thought 13:33:18 mriedem: we largely did 13:33:24 neutron port-list --device_id=server.uuid 13:33:26 we got 95% of them out of here 13:33:40 enough so that people are going to need to move to native tooling 13:33:57 yup, I think we hit a lot of them 13:34:01 if there remains an odd API call somewhere, that's probably not going to break the bank 13:34:29 and I feel like getting this right is going to be tricky enough that the only way it happens is if that's all you, johnthetubaguy, me, and alex do for the next 2 weeks 13:34:48 let's punt then, that's fine 13:34:49 which... I don't think is priority at this point in the cycle with so many other items 13:35:31 so I am hoping to get a neutron integration roadmap/plan thing for the summit, I will tag this as a possible thing to think about as part of that, but no promises, as its lower priority 13:35:32 and we should plan some time at Summit to talk through the oddities both in Nova, and with neutron / cinder 13:35:36 yea, looks like there isn't reason we need rush in newton 13:35:45 yeah, I think thats the right thing to do 13:36:08 yeah, I am thinking through how we talk to neutron and cinder APIs for live-migrate etc 13:36:36 i honestly really don't see how this is any different than what we just did last week for 2.36 13:36:37 but ok 13:37:30 let's move on 13:37:38 ok 13:37:55 mriedem: it's just my suggestion to punt. I just won't be able to put any real brain power here for a couple of weeks, as I really need to get some neutron devstack stuff happening 13:38:08 well, I think these ones we don't agree what would have if we started over, most of the other ones its clear we wouldn't have added them if they were proposed as adds next week 13:38:20 which is a stack overflow all by itself 13:38:52 did either of you read the spec? 13:39:12 bleh, let's just move on :) 13:39:39 heh 13:39:43 mriedem: no... because, I have this giant other stack 13:39:56 which was kind of my point :) 13:40:29 anything more for priorities? 13:40:41 I don't think so 13:40:56 alex_xu: imageRef one 13:40:57 and we have last two microversion patch need to review 13:41:07 gmann_: ah, thanks, yes, that one 13:41:10 #link https://review.openstack.org/#/c/338802/ 13:41:52 so the question is we agree on accept empty string, right? 13:42:00 alex_xu: i will add empty string in base schema anyways for non volume bot empty string not-allowed is checked in python code 13:42:04 boot 13:42:11 so this is all good. 13:42:25 gmann_: yea 13:42:25 for glance client cleanup i have few query 13:42:53 first is- we will remove the alternate_link from image APIs links whihc shows glance links 13:43:19 bit there are other place also we have those 13:43:33 in create_image action response header - https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L1019-L1022 13:43:34 would any of this break anyone still using glance v1? 13:43:37 if use_glance_v1=True? 13:43:45 b/c if so we should wait until that's all removed 13:43:59 it shouldn't, it's actually stuff that comes through to the user in odd ways 13:44:07 we don't test that stack though 13:44:10 alternate_link has kind of been a mess 13:44:14 yea 13:44:18 b/c we thought we wouldn't regress it 13:44:34 mriedem: right, what I'm saying, is this should have no impact there 13:44:42 should, but 13:45:00 anyway, bringing it up 13:46:02 I had uploaded a patch set to allow "revert_resize" api to recover error instance after resize/migrate. 13:46:07 so for response header location, what we want to return. ? 13:46:17 #link https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L1019-L1022 13:46:35 hanrong1: wait one more second after this topic :) 13:46:41 sorry 13:46:42 ok 13:46:43 and other place it is used is in notification instance info - https://github.com/openstack/nova/blob/master/nova/notifications/base.py#L423 13:46:48 hanrong1: no worries 13:47:26 these are two place we should think what to store/return 13:47:46 so we're going to break 2 apis? 13:47:48 gmann_: oh... wow, what an abuse of the Location header 13:48:26 ug 13:48:47 doesn't image_ref just become uuid, or is enforced to be uuid? 13:48:53 ok... so 13:48:57 but today it can be an int or something? 13:49:02 I think we can do a much smaller thing 13:49:09 my initial thought on these was to keep as it is and only go for strict input imageRef as UUID 13:49:21 gmann_: yeh 13:49:29 i mean only this patch - https://review.openstack.org/#/c/338802/ 13:49:38 thats where I was heading, just the input validation 13:49:43 I think the only thing we really care about is the validation on the field passed to us 13:49:51 and leave everything else the same 13:49:58 i do not want to take risk :) 13:50:23 and other place in virt driver - https://github.com/openstack/nova/blob/master/nova/virt/images.py 13:50:32 and at some point we need to make the Location / alternate_link use a more sensible url, because randomly picking a glance internal server is probably not what you actually want 13:50:34 which i did not get fully where those are used 13:51:02 sdague: yea 13:51:36 so we have agreement, then let's move on? 13:51:46 may eb the time we completely cleanup glance v1 stuff 13:51:46 also, use of the Location header with 202 response is totally not a thing in HTTP.... but oh well 13:52:07 sins of the past 13:52:21 sdague: ah, nice catch :) i did not notice that 13:52:43 alex_xu: i think yea. i will update that soon 13:52:52 but give me a lot of chance to learn HTTP :) 13:52:59 https://review.openstack.org/#/c/338802 - I think seems reasonable 13:53:01 gmann_: cool, thanks 13:53:08 and I think is all we want to change at this point 13:53:38 gmann_: there should be a pretty giant samples update along with that as well, right? 13:53:39 ok. 13:53:52 I'm actually confused how those work with that patch 13:53:59 sdague: no, it was just in 2.19 server sample 13:54:26 sdague: i think we do not have alternate_link for any API than image one 13:54:53 ok 13:55:31 and other links got cleanup in your patches of optimizing the host name etc in functional tests 13:55:41 5 mins left 13:55:48 I forget the time 13:55:48 alex_xu: we can move 13:55:59 #topic Open 13:56:05 hanrong1: your turn 13:56:15 #link: https://review.openstack.org/#/c/334747/ 13:56:33 I had uploaded a patch set to allow "revert_resize" api to recover error instance after resize/migrate. I want to discuss whether this modification need microversio 13:57:09 I also have a bp need review 13:57:25 https://blueprints.launchpad.net/nova/+spec/add-ratio-to-hypervisor-show 13:57:28 I think it don't need a microversion, because restful interface is not changed. 13:57:39 alex_xu / johnthetubaguy - is this something we'd backport as a bug fix? 13:57:46 hanrong1: behavior changes 13:58:20 songruixia: that will have to be proposed as a spec in ocata 13:58:21 it feels like we shouldn't backport it, and I would want to know where its supported, feels like a microversion, but it also feels borderline 13:59:28 it's a behavior change yeah 13:59:31 hanrong1: have a email 13:59:31 so needs signaling 13:59:45 do we need continue that on maillist? 13:59:45 same as tdurakov's api change to make live migratione pre-check async 13:59:51 i has post a spec #link: https://review.openstack.org/#/c/350348/ 13:59:51 https://review.openstack.org/#/c/350348/ 13:59:51 https://review.openstack.org/#/c/350348/ 13:59:51 https://review.openstack.org/#/c/350348/ 13:59:51 https://review.openstack.org/#/c/350348/ 13:59:51 due to we have 1 min left... 14:00:02 sorry , 14:00:04 sdague: you mean backport of imageRef things? 14:00:08 songruixia: then sit tight, we aren't reviewing specs for ocata right now 14:00:13 we'll do that after ocata opens up 14:00:15 alex_xu: yea, continue with maillist 14:00:19 gmann_: no, about the revert resize 14:00:22 sorry, we run out of time, let us back to nova channel or maillist 14:00:24 sdague: ok 14:00:27 #endmeeting