Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-01-26
11:42:28 sean-k-mooney gibi: i think the issue is each of the gabby test files are written such that they need to be run indepently
11:42:47 lyarwood khomesh24: that looks valid to me, did you have a patch?
11:43:53 sean-k-mooney for example https://github.com/openstack/placement/blob/master/placement/tests/functional/gabbits/shared-resources.yaml
11:44:08 sean-k-mooney needs to be run with its own placment instance
11:46:18 sean-k-mooney although that is proably not the best example since it wont conflict with others since other dont create inventoies of CUSTOM_MAGIC
11:47:27 stephenfin gibi: I just figured it out
11:47:31 stephenfin https://storyboard.openstack.org/#!/story/2008556
11:48:01 stephenfin pushing a fix shortly
11:50:27 gibi sean-k-mooney, stephenfin: thanks, then I will not dig into it
11:51:26 sean-k-mooney stephenfin: i assume that is because we are using a global object at some point and not mockign it propertly in the tests
11:52:26 stephenfin yes, the RuleDefault objects provided in placement.policies.*
11:59:24 gibi sean-k-mooney: replied in https://review.opendev.org/c/openstack/nova/+/757109 I think you missed the @retries decorator
12:00:00 khomesh24 lyarwood, no i wanted to valid first
12:07:08 lyarwood khomesh24: kk for me it's valid, I've marked the bug as confirmed
12:07:22 lyarwood khomesh24: it doesn't make sense to allow an admin to reset to active if the instance is offloaded
12:08:56 khomesh24 lyarwood, ack, i will start working on the patch then, thank you
12:12:21 gibi lyarwood, khomesh24: do we then make os-resetState smart for other non valid reality-db state combinations too?
12:13:14 gibi for me os-resetState is a blunt instrument, and the implementation also suggest that (it validates nothing)
12:13:40 lyarwood gibi: I wouldn't be opposed to it tbh, anything we can do to avoid obvious PEBKAC is good with me
12:14:19 gibi lyarwood: so when the admin reset to ACTIVE then we should call down to the compute to validate that the instance is really running happily?
12:14:54 lyarwood gibi: no I'd just sanity check the vm_state
12:15:07 lyarwood gibi: SHELVED_OFFLOADED to ACTIVE obviously isn't valid
12:16:20 lyarwood gibi: https://docs.openstack.org/api-ref/compute/?expanded=reset-server-state-os-resetstate-action-detail#reset-server-state-os-resetstate-action the docs are also wrong, you can set any valid vm_state AFAICT
12:16:21 gibi lyarwood: is it not valid as ACTIVE would need instance.host set but we assume that in SHELVED_OFFLOADED the instance.host is None?
12:16:46 lyarwood gibi: correct, any further action would just fail
12:17:07 lyarwood gibi: I don't want to block admins from fixing actual issues with instances
12:17:27 lyarwood gibi: but I can't see how allowing SHELVED_OFFLOADED to ACTIVE would help in any situation tbh
12:17:47 lyarwood gibi: lets wait for a patch to emerge and we can thrash this out on the review
12:17:52 gibi lyarwood: the api schema restricts the new state to enum': ['active', 'error']
12:17:56 lyarwood ah
12:18:12 lyarwood thanks
12:18:35 gibi lyarwood: OK, then I would not depend on SHELVED_OFFLOADED but depend on instance.host is not None, when resetting to ACTIVE
12:18:55 gibi that would catch ERROR -> ACTIVE for VM that was never scheduled to a host
12:19:05 gibi (and other edge cases, I guess)
12:20:03 gibi lyarwood: also I would add warning to the API ref that resetting state is dangerous
12:25:01 openstackgerrit Stephen Finucane proposed openstack/placement master: policy: Suppress policy deprecation warnings https://review.opendev.org/c/openstack/placement/+/771852
12:25:02 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for resource providers https://review.opendev.org/c/openstack/placement/+/760240
12:25:03 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for aggregates https://review.opendev.org/c/openstack/placement/+/760235
12:25:05 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for allocation candidates https://review.opendev.org/c/openstack/placement/+/760237
12:25:05 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for allocations https://review.opendev.org/c/openstack/placement/+/760236
12:25:07 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for inventories https://review.opendev.org/c/openstack/placement/+/760238
12:25:08 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for resource classes https://review.opendev.org/c/openstack/placement/+/760239
12:25:09 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for traits https://review.opendev.org/c/openstack/placement/+/760241
12:25:10 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for usage https://review.opendev.org/c/openstack/placement/+/760242
12:25:11 openstackgerrit Stephen Finucane proposed openstack/placement master: Allow project readers to see project-specific usages https://review.opendev.org/c/openstack/placement/+/771964
12:25:12 openstackgerrit Stephen Finucane proposed openstack/placement master: Refactor usage tests to properly test project personas https://review.opendev.org/c/openstack/placement/+/772061
12:25:13 openstackgerrit Stephen Finucane proposed openstack/placement master: policy: Remove the deprecated 'placement' rule https://review.opendev.org/c/openstack/placement/+/772333
12:25:14 openstackgerrit Stephen Finucane proposed openstack/placement master: policy: Deprecate 'admin_api' rule https://review.opendev.org/c/openstack/placement/+/772334
12:25:15 openstackgerrit Stephen Finucane proposed openstack/placement master: policy: Don't persist default rule changes in tests https://review.opendev.org/c/openstack/placement/+/772508
12:27:41 gibi sean-k-mooney: replied here too https://review.opendev.org/c/openstack/nova/+/769720
12:55:20 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for resource providers https://review.opendev.org/c/openstack/placement/+/760240
12:55:21 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for aggregates https://review.opendev.org/c/openstack/placement/+/760235
12:55:22 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for allocations https://review.opendev.org/c/openstack/placement/+/760236
12:55:23 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for allocation candidates https://review.opendev.org/c/openstack/placement/+/760237
12:55:24 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for inventories https://review.opendev.org/c/openstack/placement/+/760238
12:55:25 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for resource classes https://review.opendev.org/c/openstack/placement/+/760239
12:55:26 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for traits https://review.opendev.org/c/openstack/placement/+/760241
12:55:27 openstackgerrit Stephen Finucane proposed openstack/placement master: Implement secure RBAC for usage https://review.opendev.org/c/openstack/placement/+/760242
12:55:28 openstackgerrit Stephen Finucane proposed openstack/placement master: Allow project readers to see project-specific usages https://review.opendev.org/c/openstack/placement/+/771964
12:55:29 openstackgerrit Stephen Finucane proposed openstack/placement master: Refactor usage tests to properly test project personas https://review.opendev.org/c/openstack/placement/+/772061
12:55:30 openstackgerrit Stephen Finucane proposed openstack/placement master: policy: Remove the deprecated 'placement' rule https://review.opendev.org/c/openstack/placement/+/772333
12:55:31 openstackgerrit Stephen Finucane proposed openstack/placement master: policy: Deprecate 'admin_api' rule https://review.opendev.org/c/openstack/placement/+/772334
12:55:51 gibi sean-k-mooney: and here too https://review.opendev.org/c/openstack/nova/+/756530
13:02:26 stephenfin johnthetubaguy: If you have any time in the next fortnight, your attention would be appreciated on https://review.opendev.org/q/topic:%22secure-rbac%22+project:openstack/placement+is:open
13:09:33 sean-k-mooney gibi: you are right i did miss the retry decorator
13:11:56 sean-k-mooney gibi: and yes i know mypy is smart enough to deduce types when you initalise something directly but i dont think we should actully rely on that in general
13:12:14 sean-k-mooney gibi: that is just a prefernce however not a -1 thing
13:13:00 sean-k-mooney well i dont know i change my mind on that some times
13:13:28 sean-k-mooney i do sometimes use that to say it returns/accpets dict or list
13:20:53 gibi mypy requires you to add typehint for func signature (or it will use Any) but in function body mypy does proper deducing or will raise an error and ask you to add hints
13:21:26 gibi if the type is not clear
13:22:02 sean-k-mooney ya
13:23:56 openstackgerrit Merged openstack/nova stable/victoria: compute: Lock by instance.uuid lock during swap_volume https://review.opendev.org/c/openstack/nova/+/758730
13:26:25 sean-k-mooney gibi: +1 on both of those reviews
13:28:56 gibi thanks
13:30:17 sean-k-mooney lyarwood: bauzas: stephenfin and I are both +2/+1 on gibi's qos seriese https://review.opendev.org/q/topic:%22bp%252Fsupport-interface-attach-with-qos-ports%22+(status:open%20OR%20status:merged) do either of ye have time to review them and be the second +2+w
13:30:44 gibi would be appreciated ^^
13:58:45 gmann brinzhang: ack, with change in routes.py it should detect the new API. let me check today
14:11:51 nightmare_unreal brinzhang: can you help me with this one ? https://storyboard.openstack.org/#!/story/2008553
14:28:56 bauzas sean-k-mooney: ack, sorry for the delay, but I'll try to look at it
14:29:31 bauzas I actually promised some time for gibi :D
14:30:28 sean-k-mooney bauzas: :) while the series is longish most patches are small and self contaiend so its quite straight forward to review.
14:44:38 openstackgerrit Merged openstack/nova master: db: Compact Icehouse database migrations https://review.opendev.org/c/openstack/nova/+/758394
14:49:35 openstackgerrit Lance Bragstad proposed openstack/placement master: trivial: Remove TODO in LegacyRBACPolicyFixture https://review.opendev.org/c/openstack/placement/+/772535
15:03:10 stephenfin lbragstad: Replied on https://review.opendev.org/c/openstack/placement/+/760238
15:03:40 lyarwood sean-k-mooney: yup I can try to look today
15:15:05 lbragstad stephenfin ack - thanks
15:47:33 lyarwood gibi / dansmith ; https://review.opendev.org/c/openstack/nova/+/771837 should be ready to go now both of the tests are fixed
15:48:47 gibi lyarwood: ack
15:58:21 openstackgerrit Stephen Finucane proposed openstack/placement master: Expand legacy tests for inventories https://review.opendev.org/c/openstack/placement/+/772567
15:58:23 stephenfin lbragstad: ^
15:58:43 stephenfin That's super tedious work. I don't envy you /o\
15:58:59 lbragstad :)
15:59:11 stephenfin (and won't be pulling my +2s because they're missing, tbh. I can do without them)
15:59:30 lbragstad i'm in the middle of attempting the same for cinder - but thanks for proposing those
16:00:07 lbragstad yeah - i think that series looks good and i can propose the followup with additional coverage
16:00:15 stephenfin agreed
16:00:25 stephenfin we'll see what gmann thinks
16:05:52 dansmith lyarwood: question for you here: https://review.opendev.org/c/openstack/tempest/+/771958
16:06:00 dansmith not sure I understand what actually changed

Earlier   Later