| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-19 | |||
| 16:06:13 | stephenfin | gmann: some of the rules are using PROJECT_MEMBER_OR_SYSTEM_ADMIN but I'm seeing requests using the non-admin user rejected | |
| 16:06:35 | stephenfin | I assume I need to toggle that "enable new system-scoped policy" config option to get this working? | |
| 16:08:16 | gmann | stephenfin: with PROJECT_MEMBER_OR_SYSTEM_ADMIN check_str we have is to validate the 'system:all' for SYSTEM_ADMIN part and projectid&&member role for PROJECT_MEMBER so non admin would not pass if it is different project | |
| 16:08:33 | gmann | stephenfin: are you removing the old defaults also? if not then it should keep working | |
| 16:08:37 | gmann | let me check the patch | |
| 16:08:53 | stephenfin | gmann: okay, that's what I was expecting but it doesn't happen | |
| 16:09:19 | gmann | if you enable enforce_scope then it will start failing. as it need scope checks also | |
| 16:09:19 | stephenfin | if you could have a look, I'd be grateful :) I think it's a good idea (enabling policy by default) | |
| 16:10:19 | gmann | stephenfin: yeah, that is really good idea to run tests with actual policy. though we might need to do lot of work on test side though | |
| 16:10:23 | gmann | checking.. | |
| 16:10:38 | stephenfin | not as much as you'd fear | |
| 16:13:34 | gmann | stephenfin: if you try to do with admin (super access role) then yes no much changes but tests with real policy will be to use reader, member, admin roles. GET APIs by reader role not admin and so on | |
| 16:17:18 | stephenfin | yeah, to be fair, I've only tested with legacy policy config and I've been blasting through things with admin credentials where necessary /o\ | |
| 16:25:39 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Fix indentation in test__video_model_supported https://review.opendev.org/746979 | |
| 16:25:39 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Add libvirt version mocks in test_private_destroy_* tests https://review.opendev.org/746980 | |
| 16:25:40 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION and NEXT_MIN_{LIBVIRT,QEMU}_VERSION https://review.opendev.org/746981 | |
| 16:25:40 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_LIBVIRT_FILE_BACKED_DISCARD_VERSION https://review.opendev.org/746982 | |
| 16:25:41 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_{LIBVIRT,QEMU}_NATIVE_TLS_VERSION https://review.opendev.org/746983 | |
| 16:25:41 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_LIBVIRT_BETTER_SIGKILL_HANDLING https://review.opendev.org/746984 | |
| 16:25:42 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_LIBVIRT_VIDEO_MODEL_VERSIONS https://review.opendev.org/746985 | |
| 16:25:42 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_{LIBVIRT,QEMU}_PMEM_SUPPORT https://review.opendev.org/746986 | |
| 16:28:06 | gmann | stephenfin: ok. for legacy policy, that is ok. | |
| 16:33:25 | gmann | stephenfin: those failing test missing ADMIN_API=True, i checked admin_action failed tests. because by default it is non admin API request, sample tests need to set ADMIN_API for admin APIs - https://github.com/openstack/nova/blob/cff7382fb3de4eef7aeddc34b6ae3409cbe50ea0/nova/tests/functional/integrated_helpers.py#L1036 | |
| 16:33:54 | stephenfin | cool, that's what I've done in most cases | |
| 16:35:40 | gmann | and for GET APIs where default is admin_or_owner they are failing for missing admin fields in response which confirms this and no other policy issue | |
| 16:38:11 | stephenfin | so I'm going to run all the servers tests as admin to get those extra fields | |
| 16:38:23 | stephenfin | but anything else, I'll just drop the admin-only fields from the response | |
| 16:38:43 | gmann | stephenfin: from func sample tests? | |
| 16:38:52 | stephenfin | yup | |
| 16:39:26 | gmann | i think we should not, functional sample tests has complete response verification and we use that in api-ref also. | |
| 16:40:04 | gmann | making servers tests as admin is ok i think | |
| 16:40:36 | stephenfin | Sure, for this like 'GET /servers/{id}' | |
| 16:40:39 | gmann | and other tests also as admin if it involve admin APi | |
| 16:41:00 | stephenfin | what about things like the rescue action? | |
| 16:41:35 | stephenfin | that's currently being tested as a non-admin user but the response we've documented in an admin-type response | |
| 16:42:01 | stephenfin | I'm thinking those admin fields aren't important in that instance and we should keep showing a non-admin response | |
| 16:42:21 | stephenfin | but maybe not | |
| 16:43:04 | gmann | rescue does not add admin-only fields. you mean rebuild? | |
| 16:43:15 | stephenfin | Sorry, I mean in the response | |
| 16:43:38 | stephenfin | actually, never mind | |
| 16:43:46 | gmann | yeah in response rescue return empty dict or admin_password | |
| 16:44:15 | stephenfin | this is what I was referring to https://github.com/openstack/nova/blob/master/nova/tests/functional/api_sample_tests/test_rescue.py#L94 | |
| 16:44:23 | stephenfin | but that's only for internal validation purposes | |
| 16:44:36 | stephenfin | that sample response doesn't need admin-fields in it | |
| 16:46:03 | gmann | stephenfin: ohk i got now. yes we can keep that with non-admin. if sample is not complete then yes keep testing that with non-admin. | |
| 16:47:51 | gmann | stephenfin: even such test can just check state from response explicitly instead of comparing it with sample file response. | |
| 17:19:13 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Move single-use function to its caller https://review.opendev.org/745904 | |
| 17:19:13 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Drop '_api' suffix from placement fixture https://review.opendev.org/745905 | |
| 17:19:14 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Enable real policy fixture by default https://review.opendev.org/745906 | |
| 17:19:14 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Add and use 'GlanceFixture' https://review.opendev.org/745907 | |
| 17:19:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Remove '_FakeImageService' https://review.opendev.org/745908 | |
| 17:19:27 | stephenfin | gmann: ^ | |
| 17:19:49 | stephenfin | I might have missed one or two tests (there were 109 failures, after all) but that should be pretty correct now :) | |
| 17:21:08 | gmann | stephenfin: ok, waiting for gate result | |
| 17:31:50 | stephenfin | dansmith: melwitt: Can you folks look at https://review.opendev.org/#/c/744958/ again today? | |
| 17:32:14 | openstackgerrit | Stephen Finucane proposed openstack/nova master: compute: Add type hints for resize functions https://review.opendev.org/745341 | |
| 17:32:14 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: compute: Add more type hints for resize functions https://review.opendev.org/745675 | |
| 17:55:25 | artom | stephenfin, lyarwood, did any of you start backporting Stephen's func tests refactors to train? Or did we want to socialize that a bit first? | |
| 17:56:51 | artom | Nothing that I can find in gerrit, at any rate. Shall I start? | |
| 17:58:52 | lyarwood | artom: I've not had a chance to get back to it so feel free to start | |
| 17:59:45 | lyarwood | elod: https://review.opendev.org/#/c/746529/ - that reminds me can you take a look at this series from artom when you have time please | |
| 18:01:13 | artom | lyarwood, yep, thanks for poking elod for me | |
| 18:01:13 | dansmith | artom: I think stephenfin did what you asked here, so can you throw your +1 back on it? https://review.opendev.org/#/c/744950/4 | |
| 18:01:21 | dansmith | er, s/back// | |
| 18:06:45 | lyarwood | melwitt: https://review.opendev.org/#/c/745512/ - trivial test cleanup if you have time today btw | |
| 18:20:39 | openstackgerrit | Lee Yarwood proposed openstack/nova master: rbd: Move rbd_utils out of libvirt driver under nova.storage https://review.opendev.org/746904 | |
| 18:20:39 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.opendev.org/574301 | |
| 18:23:42 | melwitt | lyarwood: took too long to find what I was looking for, but commented on your patch that there's a fixture for doing that | |
| 18:25:52 | dansmith | so that would only work with eventlet, | |
| 18:25:53 | dansmith | which seems like an odd thing to favor | |
| 18:26:08 | yankcrime | hey folks, i think i'm hitting this bug with an instance that was created circa rocky, but now a user has stopped and is attempting to start it again for this first time under ussuri: https://bugs.launchpad.net/nova/+bug/1864020 | |
| 18:26:08 | openstack | Launchpad bug 1864020 in OpenStack Compute (nova) "libvirt.libvirtError: Requested operation is not valid: format of backing image %s of image %s was not specified in the image metadata (See https://libvirt.org/kbase/backing_chains.html for troubleshooting)" [Undecided,Fix committed] - Assigned to Lee Yarwood (lyarwood) | |
| 18:26:34 | yankcrime | if i'm reading right, it's related to libvirt 6.0 and that fix only applies to new instances that are created with this vesion of libvirt | |
| 18:30:34 | lyarwood | yankcrime: yeah I thought libvirt then relaxed this in a later release? | |
| 18:31:27 | melwitt | dansmith: yeah I guess loopingcall uses eventlet for its looping https://github.com/openstack/oslo.service/blob/master/oslo_service/loopingcall.py | |
| 18:32:56 | lyarwood | yankcrime: sorry need to drop for the day now but if this isn't resolved by a later release of libvirt then you might need to convert the disks in place to include the format details of the backing file | |
| 18:33:05 | dansmith | yeah that seems weird to me -- if we were to convert some of our stuff to real threads as we've discussed before, that'd be a problem | |
| 18:34:16 | yankcrime | lyarwood: yeah that's the conclusion i'm coming to, wanted to make sure i wasn't missing anything | |
| 18:34:37 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_LIBVIRT_FILE_BACKED_DISCARD_VERSION https://review.opendev.org/746982 | |
| 18:34:38 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_{LIBVIRT,QEMU}_NATIVE_TLS_VERSION https://review.opendev.org/746983 | |
| 18:34:38 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_LIBVIRT_BETTER_SIGKILL_HANDLING https://review.opendev.org/746984 | |
| 18:34:39 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_LIBVIRT_VIDEO_MODEL_VERSIONS https://review.opendev.org/746985 | |
| 18:34:39 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Remove MIN_{LIBVIRT,QEMU}_PMEM_SUPPORT https://review.opendev.org/746986 | |
| 18:34:58 | lyarwood | yankcrime: yup sorry, I was sure the libvirt folks backed down after we complained about this but I can't seem to find the change | |
| 18:35:14 | lyarwood | yankcrime: kashyap is the person to speak to about this, I'll follow up with him in the morning | |
| 18:35:19 | yankcrime | lyarwood: damn, fwiw i'm on 6.0.0-0ubuntu8.1~cloud0 | |
| 18:35:24 | yankcrime | lyarwood: thanks! | |
| 18:36:54 | melwitt | dansmith: yeah, if we convert then we'd have to add support to loopingcall or stop using loopingcall I guess | |
| 19:32:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Add reproducer for bug #1879878 https://review.opendev.org/744950 | |
| 19:32:52 | openstack | bug 1879878 in OpenStack Compute (nova) "VM become Error after confirming resize with Error info CPUUnpinningInvalid on source node " [Medium,In progress] https://launchpad.net/bugs/1879878 - Assigned to Stephen Finucane (stephenfinucane) | |
| 19:32:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Don't unset Instance.old_flavor, new_flavor until necessary https://review.opendev.org/744958 | |
| 19:32:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: manager: Move context manager up one level https://review.opendev.org/745674 | |
| 19:32:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: compute: Add type hints for resize functions https://review.opendev.org/745341 | |
| 19:32:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: compute: Add more type hints for resize functions https://review.opendev.org/745675 | |
| 19:33:00 | stephenfin | dansmith: Addressed your comments ^ If you could take the third one in that series too, that'd be great (split out per your request) | |
| 20:16:58 | openstackgerrit | Merged openstack/nova master: libvirt: Mock LoopingCallBase._sleep in rbd tests https://review.opendev.org/745512 | |
| 20:26:05 | mnaser | fyi, ceph jobs are breaking often and i tracked it down to https://review.opendev.org/747030 | |
| 20:26:10 | mnaser | hoping to land that when we can :) | |
| 20:33:00 | melwitt | thanks mnaser. looks like cinder core are core on devstack-plugin-ceph | |
| 21:21:56 | openstackgerrit | Merged openstack/python-novaclient master: zuul functional job: drop the custom playbooks https://review.opendev.org/746649 | |