Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-08-08
21:29:06 dansmith I know
21:31:34 melwitt jaypipes: fwiw, you might be able to solve it at the keystone level, like if you could present keystone with admin creds and then receive back a token for another user (if you have the appropriate role), then it would be transparent to nova. the context nova receives would be for the wanted user
21:32:37 melwitt like if there were a role, "Allowed to act on behalf of project X" that admin type users could be a part of
21:33:59 dansmith or write a half page doc for your users on how to boot their first instance
21:34:46 mriedem dansmith: we have a bug for that with shelve,
21:34:48 melwitt yeah, I can't remember what it's for, something automated where a headless user is doing it
21:34:49 mriedem user creates server,
21:34:51 mriedem admin shelves it,
21:34:55 mriedem user unshelves,
21:34:59 mriedem kablammo
21:35:00 dansmith boom
21:35:01 dansmith heh yeah
21:35:03 mriedem b/c user doesn't have access to the admin snapshot
21:35:08 dansmith yar
21:35:12 melwitt that's fun
21:35:27 melwitt I guess the keystone idea would solve that issue
21:35:34 mriedem i'm pretty sure it's a relatively straight-forward fix, just haven't ever had the proper motivation to work on it
21:35:41 mriedem no no no
21:35:56 mriedem when we snapshot, we make the instance owner (the user) have member access to the image
21:36:00 mriedem glance has a thing for that
21:36:30 mriedem https://specs.openstack.org/openstack/glance-specs/specs/api/v1/requesting_image_memberships.html
21:36:55 mriedem https://specs.openstack.org/openstack/glance-specs/specs/api/v1/adding_a_member_to_an_image.html
21:36:58 melwitt neat
21:37:41 mriedem https://bugs.launchpad.net/nova/+bug/1675791
21:37:41 openstack Launchpad bug 1675791 in OpenStack Compute (nova) "Instance created by demo user(non-admin), shelved by admin and unshelved by demo user --> ends up in error state " [Medium,Triaged] - Assigned to Damini Chopra (damini)
21:38:09 openstackgerrit Merged openstack/nova master: Add explicit functional-py36 tox target https://review.openstack.org/589825
21:38:19 openstackgerrit Merged openstack/nova master: Add the guideline to write API reference https://review.openstack.org/569058
21:39:55 mriedem speaking of changing stuff on unshelve https://review.openstack.org/#/c/559828/
21:40:44 melwitt you've opened the shelve bug bin
21:42:06 mriedem time to create a shelve sig
21:42:10 mriedem / support group
21:42:18 mriedem for admins that have enabled that api
21:42:32 melwitt lol
21:53:45 mriedem melwitt: dansmith: a couple of thoughts in this ocata-only func test https://review.openstack.org/#/c/588416/
21:53:57 mriedem if you wanted to make it less unit-y
21:55:21 dansmith mriedem: I definitely think this is a unit test, as I said
21:55:23 mriedem i guess you don't actually start conductor when using NoopConductorFixture
21:56:07 melwitt argh, I thought I had to write it this way to fit in with the pattern of regression tests
21:56:45 melwitt whichever one it needs to be, I always pick the wrong one :)
21:56:53 openstackgerrit Merged openstack/nova master: Remove Neutron MetaAPIProxy from cellsv2-layout https://review.openstack.org/588525
21:58:15 mriedem i'm a tad confused on the stub in the 2nd test
21:58:59 melwitt in what way?
21:59:08 mriedem it says,
21:59:09 mriedem "# Stub out the API to make the instance destroy raise InstanceNotFound,
21:59:09 mriedem # simulating if conductor already deleted it."
21:59:28 mriedem conductor only deletes the instance if conductor gets build request not found when it goes to delete the build request, right?
21:59:56 mriedem which only happens if _attempt_delete_of_buildrequest returns True right?
21:59:57 melwitt yes. I glossed over that from writing up all the other comments. O tjoml
22:00:05 melwitt *,I think
22:00:13 melwitt yes
22:00:20 mriedem so...
22:00:25 mriedem aren't the first and 2nd tests really the same thing?
22:00:40 mriedem or
22:00:45 mriedem i guess one is saying,
22:00:53 melwitt no because the first test will commit the reservations and the second one won't and will fall into the exception handling and rollback and refresh
22:00:54 mriedem the 2nd is saying,
22:01:15 mriedem between the time _lookup_instance returns the instance, another request deletes the build request, so conductor deletes the instance and then request 1 fails on instance.destroy()?
22:01:21 melwitt the first test never goes into the except InstanceNotFound block
22:01:43 melwitt yes
22:02:06 mriedem i'm not following "no because the first test will commit the reservations and the second one won't and will fall into the exception handling and rollback and refresh"
22:02:27 mriedem but both _lookup_instance and instance.destroy() are in the same try/except InstanceNotFound in _delete_while_booting
22:02:30 melwitt I just mean the first test will not cover the except InstanceNotFound block
22:02:38 mriedem so the point is, each test is covering two ways we could raise in there
22:02:41 melwitt _lookup_instance does not raise InstanceNotFound
22:02:52 melwitt it returns (None, None) if it catches InstanceNotFound
22:03:12 mriedem oh right
22:03:30 mriedem ok
22:04:37 melwitt so first case is _lookup_instance returns (None, None) because it found no instance. second case is instance was found but then instance.destroy() raises because conductor deleted it out from under us after we deleted the build request but before we got to instance.destroy()
22:05:19 melwitt *we deleted the build request, found the instance, and then it was gone in that small window when we got to destroy
22:06:09 mriedem ok
22:06:33 mriedem well, up to you on if you or dansmith want my comments addressed, if you don't care, then i don't really care much either and i'll +W
22:07:07 mriedem also,
22:07:33 mriedem you're sure you're not hitting this? https://github.com/openstack/nova/blob/stable/ocata/nova/compute/api.py#L1729
22:07:33 dansmith I don't like the test on multiple fronts,
22:07:38 mriedem b/c i see you're not using the AllServicesCurrent fixture
22:07:51 dansmith but it seems like the fix is worth getting in, nits on the test aside
22:07:54 melwitt I'd rather not have to convert them to unit tests at this point but if you want me to change it, I will
22:08:29 mriedem no i'm not talking about full on re-writing them as unit tests
22:08:39 mriedem i just meant with stuff like the AllServicesCurrent fixture and NoopConductorFixture
22:09:06 mriedem but we must not be hitting https://github.com/openstack/nova/blob/stable/ocata/nova/compute/api.py#L1729 because your change to the tests in the next patch are ok
22:09:25 melwitt yeah ... seems like it would return early because of the service version but the test does pass after the fix, which means it's getting to the quota calls somehow
22:09:45 dansmith because there are no services?
22:09:53 mriedem the api service is running
22:09:54 dansmith I don't see why we want noopconductor over just not starting it
22:10:29 dansmith but it's all just forced because this should be a unit test where it doesn't matter anyway :)
22:10:49 melwitt SORRY GUYS for my blasphemous functional test
22:10:50 mriedem i'm just surprised that fixture isn't needed is all i'm saying
22:11:03 mriedem like, why do we even need it in the first place in other func tests is my point
22:11:31 mriedem thou shalt be smoten with review nit-pickery
22:11:34 melwitt we have definitely needed it before. I'm not sure what's going on here
22:11:46 melwitt re: ServicesCurrent
22:12:11 dansmith if you have no services you get back the current I think
22:12:18 dansmith we used to return zero,
22:12:23 melwitt the api service doesn't count as a service?
22:12:39 melwitt os_apicompute
22:13:01 mriedem idk, might have something to do with wsgi-intercept usage in OSAPIFixture now
22:13:03 dansmith you're not starting an api service for real here
22:13:05 dansmith right
22:13:14 melwitt O.o
22:13:19 dansmith but anyway if this was A UNIT TEST

Earlier   Later