Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-06-22
19:13:33 sean-k-mooney i have not looked in a while but last time i did i did not see it
19:13:55 melwitt dansmith: so.... it "seems" like you would probably do a patch similar to this one https://review.opendev.org/410394
19:14:41 melwitt that ^ adds the nova-neutron interaction. and the groundwork was added in a prior patch for the nova-cinder interaction https://review.opendev.org/397399
19:15:09 dansmith ack yeah, okay
19:15:10 dansmith well, I guess I'll start by arguing that we shouldn't need admin to do this
19:15:49 dansmith melwitt: and just to stitch that stuff together, you think that when we do admin=True to the neutronclient, we're now getting the service user's auth token?
19:16:01 dansmith the logic on L135 there is a bit confusing
19:16:05 melwitt sean-k-mooney: we enable it in nova-next https://github.com/openstack/nova/blob/f1ebc15dfc8ffb7f23b2cb9879f0ca9376931a90/.zuul.yaml#L180
19:16:23 sean-k-mooney melwitt: ah ok but not in dansmith's job https://zuul.opendev.org/t/openstack/build/18e4701c1a374bf09269778479160f25/log/controller/logs/etc/nova/nova-cpu_conf.txt
19:16:54 sean-k-mooney there is no service_user group in the config
19:17:07 melwitt yeah you'd have to set the env var for the job
19:17:58 sean-k-mooney could you just add the glance credetials like we do for neutron https://zuul.opendev.org/t/openstack/build/18e4701c1a374bf09269778479160f25/log/controller/logs/etc/nova/nova-cpu_conf.txt#43-50
19:18:36 dansmith sean-k-mooney: that's what melwitt said above
19:18:40 sean-k-mooney service_user support was more fo the case where i am doing a long running request but my user token expired so we fallback to a admin service user instead
19:18:59 dansmith and I'm guessing the answer is yes, but I'd like to not have to do that (meaning not need to have admin for this)
19:19:57 sean-k-mooney ya given you dont own the image however im not surprised that glance is unhappy
19:20:14 sean-k-mooney you could argue that if its a public image then maybe this should be allowed
19:20:17 melwitt dansmith: yeah... agreed it looks confusing. but I think yeah, passing admin=True is having it load the auth plugin from conf, which presumably will pick up the [service_user] config section
19:20:34 dansmith sean-k-mooney: it's public and the API is letting me do the operation
19:20:42 sean-k-mooney melwitt: i dont think it will by default
19:21:08 sean-k-mooney dansmith: as in the api is accepting the import
19:21:14 dansmith right
19:21:22 sean-k-mooney ya so it feels like a glance but
19:21:25 sean-k-mooney *bug
19:21:40 sean-k-mooney but for now you might need to use admin to work around it
19:21:43 dansmith I hath already filed it thusly
19:22:16 sean-k-mooney :)
19:24:00 sean-k-mooney ok so ya it looks like we have the service auth support in the image module https://github.com/openstack/nova/blob/master/nova/image/glance.py#L68
19:25:54 sean-k-mooney without modifying noava code with admin=true on that call however its not going to elevate unless the token is expired so i guess you will have to do that too which kind of sucks
19:26:16 sean-k-mooney i mean i guess you can do that in the DNM patch
19:26:39 dansmith just setting admin=True on the nova context isn't going to do it
19:26:49 dansmith I'd have to actually get an admin-granted token from keystone
19:30:13 sean-k-mooney not on the context but when you create the client cant you just pass admin=true .e.g get_cline(ctx, admin=true)
19:30:48 sean-k-mooney like we do with neutron https://opendev.org/openstack/nova/src/commit/f5f7c2540150c7ee7640c834d5caec31b3f5a7ab/nova/network/neutron.py#L397
19:32:31 sean-k-mooney although that is a custom get_client function https://opendev.org/openstack/nova/src/commit/f5f7c2540150c7ee7640c834d5caec31b3f5a7ab/nova/network/neutron.py#L234-L257
19:34:44 sean-k-mooney i guess not the glance module does not ever use admin currently so it does not pass it to https://opendev.org/openstack/nova/src/commit/f5f7c2540150c7ee7640c834d5caec31b3f5a7ab/nova/image/glance.py#L60
21:30:02 openstackgerrit Dan Smith proposed openstack/nova master: DNM: Try to make a glance multistore job https://review.opendev.org/734184
22:01:27 sean-k-mooney dansmith: oh you fixed it in glance by having it constuct the task factory with an admin context. that is much better then working around it in nova with an admin context
22:03:46 sean-k-mooney having the user download and reupload the image does seem very iniffiecnt.
22:04:34 sean-k-mooney a alternitive would be for the copy image api to create a new iamge which the current user now owns form the old image but that also does not fit with the current import api in my view
22:04:56 sean-k-mooney so ya i think your patch makes sense
22:09:08 dansmith sean-k-mooney: well, that's mostly just a minimal hammer approach to get past this block (I think).. as I hedged in the commit message, it may very well be that we should be only constructing the image pool with that admin context, or something more detailed
22:09:33 dansmith I haven't chased all the implications of doing this, I just put it up to try to move on and so someone can show me what the right way is, if indeed the user is supposed to be allowed to do this
22:09:41 dansmith totally possible that someone will say it should be admin-only
22:10:04 sean-k-mooney ya although the api in principal should have determined if yo are allowed do something before you get to that point
22:10:05 dansmith but you know, best way to get something done on the internet is to do it wrong so someone will fix it out of anger :)
22:10:09 dansmith agreed
22:10:17 dansmith if not this, then the api needs more checks
22:11:19 sean-k-mooney making image copy admin only would be strange as the other import methods are not and i think it would be the same policy endpoint?
22:12:17 dansmith well, it's a different case I think
22:12:32 dansmith the other import methods are for actually importing the image
22:12:41 dansmith this import being used for copy is a little bit weird,
22:13:08 dansmith because for the others, you wouldn't have one person create the image and another provide its data generally,
22:13:10 dansmith but that's a little bit of what this is,
22:13:19 dansmith but since it's controlled enough I would think that allowing one user that can use an image to copy it to another store in the system is fine,
22:13:30 dansmith just like I said.. I could download and re-upload it myself to get the same effect
22:13:40 sean-k-mooney i mean by extening the import workflow to me i think that implies that they are treating glance as just another data source like a url or file
22:13:44 dansmith and of course, if we want to limit some users from being able to do this, we need a more fine-grained policy knobv
22:13:56 dansmith correct,
22:14:25 dansmith but for the other import mechanisms, you wouldn't want a non-admin-or-owner to be able to do the import from-url or whatever
22:14:37 dansmith which maybe means we should only do this admin context thing if we're doing copy-to-store, I dunno
22:14:42 dansmith but I'll let them opine
22:14:55 sean-k-mooney dansmith: well not for an image they can see but not own
22:15:58 sean-k-mooney if i was to redsign this i would be tempted to invert the workflow and make the copy part of get
22:16:35 sean-k-mooney e.g. if i try to get an image form a store where it is not currently present have it be copied on the backend in paralle to streaming it to the user
22:17:01 dansmith personally I think that this should be a PUT /images/foo {'stores': ['new-store', 'existing-store']
22:17:25 dansmith I think they had a task-based approach to image importing so they put this in there since it was the minimal amount of work
22:17:36 dansmith and this is what you get by bending one thing to do another
22:17:41 sean-k-mooney as in an image action
22:18:14 sean-k-mooney or create a new image form 'existing-store'
22:18:18 dansmith the move-on-demand streaming thing fundamentally won't work for the rbd case, which is the primary reason for this :)
22:18:42 sean-k-mooney dansmith: are there billing implication to this by the way
22:18:51 dansmith create a new image just creates an explosion for no reason.. we _want_ this to be the same image with multiple locations, else we lose the affinity
22:18:58 dansmith dunno
22:18:58 openstackgerrit Ghanshyam Mann proposed openstack/nova stable/stein: Make greande jobs n-v for EM and oldest stable https://review.opendev.org/737332
22:19:02 sean-k-mooney if you upload an image as public and i imported are you billed more?
22:19:25 dansmith no idea
22:19:27 sean-k-mooney i guess normal users cant upload public images
22:19:40 sean-k-mooney the can uplload shared images but public i think is admin only
22:21:01 sean-k-mooney ya at least on my kolla install my non admin account can only select private shared or community
22:21:17 sean-k-mooney public visablity is only avaiable in the admin role
22:21:48 sean-k-mooney im not really sure what the difference is between shared, comunity and public
22:22:50 sean-k-mooney ah https://wiki.openstack.org/wiki/Glance-v2-community-image-visibility-design#Visibility_Semantics
22:23:18 gmann melwitt: i am trying to test the stable gate(stein) with the fix on legacy base job, let's see if that fix the things - https://review.opendev.org/#/c/737332/3
22:24:44 gmann problem is that, neither devstack nor devstack-gate install virtualenv on subnode
22:24:56 sean-k-mooney gmann: that was recently changed
22:25:48 gmann since last week it is failing, devstack stable branch and neutron-grenade jobs are fixed but we have nova multinode jobs legacy one
22:25:51 sean-k-mooney http://lists.openstack.org/pipermail/openstack-discuss/2020-June/015204.html
22:26:07 gmann sean-k-mooney: yeah, image update
22:26:59 sean-k-mooney didnt infra plan to fix all jobs
22:27:13 sean-k-mooney they had planned to propose patches to the base jobs
22:27:27 gmann :), no. legacy jobs are always less priority to fix.
22:27:58 melwitt gmann: cool thanks, I'll keep an eye on it
22:27:59 gmann zuulv3 native are easy to fix and add those roles wherever needed. like ensure-tox etc
22:33:51 gmann sean-k-mooney: and it depends on failure too, like 'tox not found' failure due to image updates needs to be fixed on wherever needed not in base job - #3 in this http://lists.openstack.org/pipermail/openstack-discuss/2020-June/015559.html
22:34:10 gmann tox issue also started happening in neutron, horizon, and few more repo
22:34:39 gmann so we discussed not to fix in devstack base job instead on failure side.
22:35:33 sean-k-mooney ya the whitebox-tempest-plugin job also broke but we fixed it
22:35:53 sean-k-mooney that is zuulv3 so we just added ensure-pip
22:36:02 gmann putting everything in base job can overload the jobs who does not need these tools.

Earlier   Later