| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-19 | |||
| 14:50:00 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Provide default 'host_info' for '_get_connection' https://review.opendev.org/746942 | |
| 14:50:00 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Add 'start_compute' helper https://review.opendev.org/746943 | |
| 14:50:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Use tempdir for CONF.instances_path https://review.opendev.org/746944 | |
| 14:50:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Clean up PCI tests https://review.opendev.org/746945 | |
| 14:50:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Add resize tests for PCI, SR-IOV servers https://review.opendev.org/746946 | |
| 14:50:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conductor: Don't use setattr https://review.opendev.org/746947 | |
| 14:50:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Add test for SR-IOV neutron ports https://review.opendev.org/746948 | |
| 14:50:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: fixtures: Handle binding of first port https://review.opendev.org/746949 | |
| 14:50:04 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Add live migration tests for PCI, SR-IOV servers https://review.opendev.org/746950 | |
| 14:50:09 | stephenfin | sean-k-mooney: Ta-da ^ | |
| 14:50:21 | lyarwood | dansmith: yeah understood thanks | |
| 14:50:37 | stephenfin | If you can think of ways I can improve those live migration tests to highlight bugs, lemme know | |
| 14:51:09 | stephenfin | tbh I was expecting the resize test to fail because I thought you'd said that was broken, but clearly not. Maybe I'm thinking of another move operation | |
| 14:52:14 | sean-k-mooney | stephenfin: nice | |
| 14:52:30 | sean-k-mooney | ill start working my way through them | |
| 14:52:51 | stephenfin | as for why I was seeing the HTTP 500 for NoValidHost, it's the use of the CastAsCall fixture | |
| 14:52:54 | sean-k-mooney | gibi_pto_24th: wehn your back ^ will be of interest | |
| 14:53:34 | sean-k-mooney | stephenfin: resize is broken if it does a retry i think | |
| 14:54:14 | sean-k-mooney | stephenfin: tere are edgecases that fail for all move operations except live_migration with vnic_type=direct | |
| 14:54:29 | stephenfin | sean-k-mooney: Feel free to stick in comments. I'm happy to keep bulking out these tests | |
| 14:54:47 | stephenfin | given the hardwork is done now, I think | |
| 14:54:55 | sean-k-mooney | so i think it will be nice to assert the correct behavior of gibis attach/detach feature | |
| 14:55:41 | sean-k-mooney | stephenfin: but ya i have a list in the downstream emial i sent | |
| 14:56:24 | sean-k-mooney | stephenfin: one example of something that is broke is we do not update the neutron port mac address for pf migration | |
| 14:56:52 | stephenfin | Cool. I can assert that kind of stuff in the test | |
| 14:56:54 | sean-k-mooney | that is broken for both cold and live migration | |
| 14:57:15 | sean-k-mooney | detach on livem migation with PFs is also broken | |
| 14:57:39 | stephenfin | can you attach PFs via neutron? | |
| 14:57:47 | sean-k-mooney | not after the vm is booted | |
| 14:57:49 | sean-k-mooney | but on boot yes | |
| 14:57:57 | sean-k-mooney | vnic_type=direct-phsyical | |
| 14:58:28 | sean-k-mooney | no it was rammed through as a feature without accounting form a bunch of basic edgecease | |
| 14:58:44 | sean-k-mooney | and broke backwards compatiablei of the pci alias | |
| 14:59:06 | stephenfin | sounds like CPU pinning and PCI in general | |
| 15:01:40 | sean-k-mooney | not really | |
| 15:14:38 | iurygregory | sean-k-mooney, hey I did the changes directly to the node (in setup time) and at least all non uefi/multinode jobs are green =) | |
| 15:23:05 | sean-k-mooney | cool | |
| 16:05:32 | stephenfin | gmann: suuuuper dumb question. I'm attempting to enable policy by default in all tests but seeing policy failures https://review.opendev.org/#/c/745906/ | |
| 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 | |