| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-08 | |||
| 21:24:54 | dansmith | what about when they rebuild/resize/etc? | |
| 21:24:59 | melwitt | after the admin creates it, no more admin involved | |
| 21:25:34 | dansmith | which means they can't rebuild if the admin built with an image they can't access yeah? | |
| 21:25:38 | jaypipes | dansmith: it's a "let's have the admin hand-hold you for a little while, then after that, you can use it" sort of anti-feature. | |
| 21:25:46 | melwitt | dansmith: yeah | |
| 21:25:52 | dansmith | jaypipes: yes, anti-feature is a good name for it :) | |
| 21:26:02 | melwitt | yeah. I can't really remember why they want it | |
| 21:26:19 | dansmith | jaypipes: without a lot of inbuilt support for that workflow, it sounds like the kind of thing that will always have a million caveata | |
| 21:26:22 | dansmith | like shelve | |
| 21:26:58 | melwitt | because admin can act upon all user's instances already, except for create, if I'm remembering correctly | |
| 21:27:07 | melwitt | that is, an admin can delete a normal user's instance, etc | |
| 21:27:40 | dansmith | well, that's a terrible reason, but I know what you mean :) | |
| 21:27:59 | melwitt | haha, sorry, wasn't using it as a reason | |
| 21:28:55 | melwitt | I mentioned it in the context of, I can't remember why they need it for create, being that admin can already do everything else to normal user instances | |
| 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 | 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:37:41 | mriedem | https://bugs.launchpad.net/nova/+bug/1675791 | |
| 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 | # simulating if conductor already deleted it." | |
| 21:59:09 | mriedem | "# Stub out the API to make the instance destroy raise InstanceNotFound, | |
| 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 | dansmith | I don't like the test on multiple fronts, | |
| 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: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 | |