Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-07
16:22:26 gmann true
16:22:28 artom sean-k-mooney, yeah, I got it completely wrong and panicked for nothing
16:22:45 sean-k-mooney artom: hehe k
16:23:17 dansmith gmann: okay, so the next test after that.. is changing the policy it looks like?
16:23:22 gmann dansmith: i think we should test those policy against each context. we can hack the first policy check to allow-all to verify the second.
16:24:01 gmann dansmith: yeah changing the first policy, so that we can verify the second enforced policy
16:24:06 sean-k-mooney im going to see if my second attempt to get shopping without spenindg 30 mins queing to get in to the shop works so ill be back in an hour too since ill be cooking dinner in ither case
16:24:07 dansmith gmann: I'm not checking both, I'm checking one or the other.. maybe I should just push up what I have for discussion and work on the tests after we agree?
16:24:28 gmann dansmith: yeah, because those tests are with old order of policy
16:24:56 gmann once we have correct order of policy enforcement then we can better judge how to tests
16:25:03 dansmith right okay.. let me remove that last test since it's testing the serial checking of policy anyway
16:25:04 dansmith ack
16:27:32 openstackgerrit Merged openstack/nova master: Add test coverage of existing flavor_manage policies https://review.opendev.org/714814
16:28:30 sean-k-mooney oh while i think if it we have a but with how we do validation of numa paramater if we have an automatic numa toplogy. ill try an file it when i get back but basically we ignore the hw:nuam_cpu and hw:numa_mem options if you dont have multiple numa nodes to you can set things that refer to cells that dont exist e.g hw:numa_mem.1=512 when you only have one numa node(node 0) or possible no numa nodes i
16:28:32 sean-k-mooney havent fullly check how broken it is.
16:29:11 sean-k-mooney i think the code that reads that extra spec just does not run if you dont have hw:numa_nodes>=2
16:29:46 stephenfin lyarwood: You have two merge conflict issues in the https://review.opendev.org/#/q/topic:bp/virt-bfv-instance-rescue series, fyi https://review.opendev.org/#/q/topic:bp/virt-bfv-instance-rescue
16:30:48 lyarwood stephenfin: looking
16:31:06 lyarwood how on earth
16:35:32 openstackgerrit Balazs Gibizer proposed openstack/python-novaclient master: Microversion 2.83 - Add more filters for the nova list command https://review.opendev.org/713089
16:36:50 openstackgerrit Lee Yarwood proposed openstack/nova master: compute: Extract _get_bdm_image_metadata into nova.utils https://review.opendev.org/705212
16:36:50 openstackgerrit Lee Yarwood proposed openstack/nova master: api: Introduce microverion 2.87 allowing boot from volume rescue https://review.opendev.org/701430
16:36:51 openstackgerrit Lee Yarwood proposed openstack/nova master: libvirt: Support boot from volume stable device instance rescue https://review.opendev.org/701431
16:37:40 openstackgerrit Dan Smith proposed openstack/nova master: Separate update and swap volume policies https://review.opendev.org/711194
16:38:32 gibi stephenfin, gmann, brinzhang: made --config-drive a pass through option in https://review.opendev.org/713089 to decuple that change from the bugfix https://bugs.launchpad.net/nova/+bug/1871409
16:38:34 openstack Launchpad bug 1871409 in OpenStack Compute (nova) "cannot filter the list servers to show only those that does not have config drive" [Medium,In progress] - Assigned to Balazs Gibizer (balazs-gibizer)
16:40:06 stephenfin gibi: What kind of values can the user actually pass through at the moment?
16:40:20 gibi stephenfin: API schema allows any string
16:40:28 gibi stephenfin: and matches it against the DB value
16:40:43 gibi which is also a String(255)
16:41:01 gibi (the API schema does not allow empty string though)
16:41:09 stephenfin right, but what values will do something meaningful? Do we only ever store a string-ified True/False value in there?
16:41:39 gibi stephenfin: we definitely store "True" and "", but I'm not sure if we store other values
16:42:36 gibi at some point in the past we allowed to store a uuid there
16:42:47 gibi https://bugs.launchpad.net/python-novaclient/+bug/1314395
16:42:49 openstack Launchpad bug 1314395 in python-novaclient "Documentation for config_drive boot parameter is incorrect" [Undecided,Fix released] - Assigned to Jason Dunsmore (jasondunsmore)
16:43:07 gibi so in some crazy old deployment the db could contain a uuid
16:43:24 gibi old, mean existed for a long time but upgraded to new nova code
16:44:56 stephenfin gibi: We haven't let you create such a server in quite some time though https://github.com/openstack/nova/blob/bae8b1306eb/nova/compute/api.py#L532-L545
16:45:11 stephenfin What I'm getting at is that it doesn't seem like there's much point allowing the user to pass through an arbitrary string, since the users that can use it are vanishingly small
16:45:33 stephenfin So I wonder if we should just allow true/false and add a TODO that false doesn't actually work until that bug is fixed?
16:45:59 stephenfin specifically, we should use a '--[no-]config-drive' option pair
16:47:04 gibi the API allows now to send in any string in the query param config_drive, when we fix that we need to bump the API microversion. I guess that will be a good signal to change the client to only allow true/false to be passed
16:48:12 stephenfin True. The client doesn't have to match the server 100% though. We could limit the user client side, even if it's currently a free-for-all server side
16:48:39 gibi true
16:49:02 stephenfin I just think allowing '--config-drive foo' is daft, since no ones going to use it, and '--config-drive [True|False]' is worse than '--[no-]config-drive'
16:49:54 gibi I have to finish for today and we have like 2 days to land the client patches. I can look at it tomorrow again
16:50:20 stephenfin Yeah, me too. The dog's not going to walk itself :)
16:50:23 gibi I free I buring time on this that would be better burnt on review
16:50:30 gibi I feel
16:50:40 gibi but meh.
16:50:42 gibi see you tomorrow
16:50:48 stephenfin o/
17:01:47 stephenfin gmann: Am I correct here? https://review.opendev.org/#/c/709955/3/nova/api/openstack/compute/volumes.py@401
17:09:57 gmann stephenfin: yeah. scope_type are not override-able so passing instance project_id is no use there
17:10:15 stephenfin Sweet. Thanks, gmann
17:10:45 gmann stephenfin: for few server actions like migration, we default them to SYSTEM_ADMIN and passing the project_id also by keeping scope_type as ['system', 'project'] so that operator can give access to project scope role also if they want
17:11:09 gmann but when scope_type itself is 'system' only then no use of project_id
17:37:41 gmann dansmith: what you think on this. checking 'update' policy always and then swap if request is swap - https://review.opendev.org/#/c/711194/14/nova/api/openstack/compute/volumes.py@478
17:38:24 openstackgerrit Merged openstack/nova master: Introduce scope_types in os-flavor-manage https://review.opendev.org/714818
17:40:42 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing server topology policies https://review.opendev.org/717524
17:45:57 dansmith gmann: that's an option too yeah. I guess the reason that seems less good is that, unless we check, in order to swap you have to have both permissions.. we might as well make one the superset and not require you to have both explicitly
17:46:06 openstackgerrit Merged openstack/nova master: Add new default roles in os-flavor_manage policies https://review.opendev.org/714819
17:48:11 gmann dansmith: yeah, i am thinking doing PUT is always need 'update' policy permission like we do in unlock. first unlock policy permission and then additional unlock_override permission.
17:49:08 dansmith okay, seems *more* confusing to me, but consistency is good, so if theres already precedent that's fine
17:50:01 gmann or if we think the use case of override i mentioned is not valid/make sense then we can keep the same way it is currently
17:50:20 dansmith gmann: does that mean we require update auth for <2.85 as well? meaning I can just put the update policy check always (i.e. remove it from under the else)?
17:50:50 gmann because it is swap operation so i am not sure interpreting it more user side cases is required or not
17:51:21 gmann dansmith: ah no, after >2.85 only and before it is always swap-only policy
17:51:43 dansmith why/
17:52:18 gmann because we are changing default means new policy itself for new operation which is allowed after 2.85
17:52:26 dansmith won't the user, who doesn't know anything about policy, wonder why they can do swap with 2.1, but cannot do swap with 2.85, because the admin has not given them update perms yet?
17:52:43 gmann if we keep default same as old 'update' policy then it is ok
17:53:47 dansmith .../
17:53:49 gmann dansmith: without overeride policy they get permission automatically but if overriding the swap policy then even no-deprecation is issue
17:54:06 dansmith I don't understand what you're saying
17:54:45 dansmith if the policy defaults are kept, then if I roll out ussuri, users who should have swap permissions will lose that ability with microversion 2.85 until I grant them update permission
17:55:05 dansmith but they would continue to be able to do it with 2.1
17:55:16 dansmith however, with what I have, they will see no change in behavior
17:55:43 gmann with defaults they are always have update permission as it is subset of swap default.
17:55:59 gmann with defaults, there is no issue on your proposal
17:56:37 gmann i am thinking if uses has override the old swap policy and want to do the same for new update policy
17:57:07 gmann if we consider either case default or override - swap is always supeset then it is ok
17:58:28 gmann but yes, we can not handle all cases in this case
17:59:56 gmann dansmith: i think it is ok with your approach and we update policy doc that for swap + update we only check swap policy which is expected to be superset of 'update'
18:00:25 dansmith okay
18:00:36 dansmith will you comment on the patch about that?
18:00:45 gmann doing,
18:00:53 openstackgerrit Merged openstack/nova master: Pass the actual target in os-flavor-manage policy https://review.opendev.org/714822
18:01:04 dansmith thanks
18:10:26 gmann dansmith: done. added comment on tests cases, few are not needed and does not test much
18:32:26 openstackgerrit Merged openstack/nova master: Fix resume server policy to be admin_or_owner https://review.opendev.org/717561
18:32:34 openstackgerrit Merged openstack/nova master: Add test coverage of existing suspend server policies https://review.opendev.org/717554
18:32:42 openstackgerrit Merged openstack/nova master: Introduce scope_types in suspend server https://review.opendev.org/717582
18:32:50 openstackgerrit Merged openstack/nova master: Add new default roles in suspend server policies https://review.opendev.org/717583
18:32:57 openstackgerrit Merged openstack/nova master: Introduce scope_types in limits policy https://review.opendev.org/715680
18:33:05 openstackgerrit Merged openstack/nova master: Add new default roles in limits policies https://review.opendev.org/715760
18:33:11 openstackgerrit Merged openstack/nova master: Add test coverage of existing server group policies https://review.opendev.org/717173
18:39:57 openstackgerrit Ghanshyam Mann proposed openstack/nova master: DNM testing enforce new defaults flag https://review.opendev.org/717945
18:40:11 openstackgerrit Ghanshyam Mann proposed openstack/nova master: DNM testing enforce new defaults flag https://review.opendev.org/717945

Earlier   Later