Earlier  
Posted Nick Remark
#openstack-nova - 2018-04-25
20:54:46 lbragstad so - i think the TL;DR is that rule:context_is_admin is used
20:56:26 lbragstad specifically when nova's middleware builds a context object, it'll parse the request environment headers, use oslo.context to get an object, then use oslo.policy's authorize methed (without a target?) to determine if context_is_admin is true
20:56:34 lbragstad based on the alias
20:56:56 lbragstad and then you set that to the ctx.is_admin attribute
20:58:51 openstackgerrit Merged openstack/nova master: Functional test: cold migrate to compute down https://review.openstack.org/496280
20:59:06 efried mriedem: I'm not smart enough to understand why this changes anything: https://review.openstack.org/#/c/563732/2/nova/compute/api.py
21:01:59 openstackgerrit Matt Riedemann proposed openstack/nova master: Add a warnings filter for inspect.getargspec usage https://review.openstack.org/564343
21:02:14 openstackgerrit Jay Pipes proposed openstack/nova master: support multiple member_of qparams https://review.openstack.org/561315
21:03:51 mriedem efried: because before it you could specify bdms without a boot_index
21:04:31 efried mriedem: But those would be excluded from the boot_indexes list, nah?
21:04:49 mriedem right
21:05:10 mriedem for context you need to look at what _validate_bdm used to do https://review.openstack.org/#/c/524208/1/nova/compute/api.py
21:05:15 mriedem if 0 not in boot_indexes or not _subsequent_list(boot_indexes):
21:05:30 mriedem mdbooth's change was to clean up and prettify the right side of that conditional
21:05:44 mriedem and b/c we had 0 test coverage for the left side, it was dropped mistakenly
21:06:10 lbragstad mriedem: http://paste.openstack.org/raw/719958/ might be an idea of how to get rid of the context_is_admin alias
21:06:13 mriedem rather than revert that outright, i went with the suggestion from stephenfin in that change to keep the pretty right side, but still enforce the left side
21:06:25 efried mriedem: But range(0) is []
21:06:49 lbragstad by bumping the processing of "is_admin" up in the chain and actually using the constructor of oslo.contexts RequestContext object
21:06:58 lbragstad (since it supports is_admin)
21:08:45 efried mriedem: So was "0 not in boot_indexes or" supposed to be added back in?
21:09:03 mriedem lbragstad: so that probably lets us drop this https://github.com/openstack/nova/blob/6d22567b9445e595f4c55627ea784801585a6e41/nova/tests/fixtures.py#L743
21:09:18 mriedem efried: that's essentially what i'm adding back in yes
21:09:24 mriedem without a full revert
21:09:42 lbragstad maybe
21:09:51 lbragstad it looks like nova still uses is_admin
21:09:54 lbragstad on the context object
21:10:21 lbragstad my diff would be a way to remove the extra alias between context_is_admin and role:admin
21:10:33 efried mriedem: okay, but I don't think you are. Because if boot_indexes is [], that `any` will fail.
21:10:43 efried Out[17]: False
21:10:43 efried In [17]: any(x != y for x, y in enumerate(boot_indexes))
21:10:43 efried In [16]: boot_indexes = []
21:10:43 efried mriedem:
21:11:17 lbragstad instead, if you're hardcoding operations to a role name, you can check for it in the headers that auth_token gives you and then pass values to RequestContext's constructor directly
21:11:29 mriedem efried: and that's ok if you're not using bdms at all
21:11:41 lbragstad but you might be able to remove https://github.com/openstack/nova/blob/master/nova/policy.py#L172-L181
21:12:13 lbragstad and save an enforcer trip
21:12:18 efried mriedem: Then the commit message isn't right. You're not changing anything logically afaict.
21:13:05 mriedem lbragstad: ok that's a bit over my head atm, since i'm doing a couple of things at once - this seems like something we could do separately and then i could build on it when adding the placement policy stuff
21:13:11 mriedem i'm just unsure of the implications of making that change
21:13:52 lbragstad does placement get context objecst from nova/context?
21:13:59 lbragstad or does it build those on it's own?
21:14:14 mriedem nova.context
21:14:18 lbragstad ok
21:14:58 lbragstad let me try something locally and i'll post a review with my thoughts, that might be easier to follow
21:16:37 mriedem efried: looks like in PS2 i changed something to fix one problem but broke what i was trying to do, so looking at it again
21:17:59 efried mriedem: PS1 looks samely broken to me.
21:18:47 efried mriedem: but the test failures are weird. Was there a separate fix for that?
21:19:37 mriedem the billions of test failures in PS1 were because it now made bdms required
21:19:52 mriedem i was fixing that in PS2, but apparently broke the test i was adding
21:19:58 mriedem fixing now
21:20:02 mriedem please hold dear caller
21:31:04 arvindn05 while you guys are on the topic of testings....are there any guidelines on test cases that they should test only one scenario?\
21:31:31 arvindn05 if i have a bunch of tests that i can make in the same test method....is that prefered?
21:31:52 arvindn05 obviously only closely related tests
21:31:55 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix dropped check for boot_index 0 in _validate_bdm https://review.openstack.org/563732
21:32:27 mriedem arvindn05: it's generally best to split them out into separate test cases
21:32:51 mriedem it's fine to use a common method if necessary with param overrides to control behavior
21:32:53 mriedem unless it gets crazy
21:34:15 arvindn05 mriedem: cool... i used that approach...number of lines increased though...hopefully wont scare away potential reviewers :)
21:47:19 openstackgerrit Lance Bragstad proposed openstack/nova master: Attempt to simplify elevated authorization https://review.openstack.org/564349
21:47:28 lbragstad mriedem: ^
21:54:16 mriedem thanks
21:58:08 openstackgerrit Eric Fried proposed openstack/nova master: WIP: placement: Granular GET /allocation_candidates https://review.openstack.org/517757
21:58:09 openstackgerrit Eric Fried proposed openstack/nova master: placement: Object changes for granular https://review.openstack.org/564351
22:09:01 openstackgerrit Eric Fried proposed openstack/nova master: placement: Object changes for granular https://review.openstack.org/564351
22:09:02 openstackgerrit Eric Fried proposed openstack/nova master: WIP: placement: Granular GET /allocation_candidates https://review.openstack.org/517757
22:11:15 openstackgerrit Matt Riedemann proposed openstack/nova master: libvirt: use dest host vif migrate details for live migration https://review.openstack.org/551370
22:11:16 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: compute: use port binding extended API during live migration https://review.openstack.org/551371
22:11:17 openstackgerrit Matt Riedemann proposed openstack/nova master: Port binding based on events during live migration https://review.openstack.org/434870
22:11:18 openstackgerrit Matt Riedemann proposed openstack/nova master: conductor: use port binding extended API in during live migrate https://review.openstack.org/522537
#openstack-nova - 2018-04-26
00:06:49 openstackgerrit Merged openstack/nova master: doc: Adds Hyper-V PCI passthrough details https://review.openstack.org/510467
00:07:05 openstackgerrit Merged openstack/nova master: api-ref: mark block_device_mapping_v2.boot_index as required https://review.openstack.org/563728
00:35:12 openstackgerrit Zack Cornelius proposed openstack/nova-specs master: Libvirt file backed memory https://review.openstack.org/563704
01:00:19 openstackgerrit Merged openstack/nova master: xenapi: handle InstanceNotFound in detach_interface() https://review.openstack.org/562838
01:14:17 yikun_ melwitt, OK, Thanks, I will update soon.
01:33:36 openstackgerrit Yikun Jiang (Kero) proposed openstack/nova master: ServerActionsSampleJsonTest refactor https://review.openstack.org/561819
01:33:37 openstackgerrit Yikun Jiang (Kero) proposed openstack/nova master: Add host/hostId to instance action events API https://review.openstack.org/557145
01:41:15 openstackgerrit Yikun Jiang (Kero) proposed openstack/nova master: Support inherit api samples in ApiSampleTestBase https://review.openstack.org/562547
01:53:05 openstackgerrit Merged openstack/nova master: Fix dropped check for boot_index 0 in _validate_bdm https://review.openstack.org/563732
06:52:44 openstackgerrit Zhenyu Zheng proposed openstack/nova master: WIP new migration threads control https://review.openstack.org/563505
07:41:14 bauzas good morning folks
07:45:56 kashyap Morning
07:50:47 openstackgerrit wanghongtao proposed openstack/nova master: Fix the metadata re to match the unicode https://review.openstack.org/536236
07:53:34 openstackgerrit Takashi NATSUME proposed openstack/nova master: Remove mox in test_compute_api.py https://review.openstack.org/564415
07:54:42 openstackgerrit Takashi NATSUME proposed openstack/nova master: Remove mox in test_compute_api.py (1) https://review.openstack.org/564415
08:13:54 openstackgerrit Jianghua Wang proposed openstack/nova master: XenAPI: define a new image handler to use vdi streaming https://review.openstack.org/486475
08:23:49 openstackgerrit Balazs Gibizer proposed openstack/nova master: Enhance service restart in functional env https://review.openstack.org/512552
08:23:51 openstackgerrit Balazs Gibizer proposed openstack/nova master: cleanup evacuated instances not on hypervisor https://review.openstack.org/512623
08:23:51 openstack bug 1724172 in OpenStack Compute (nova) "Allocation of an evacuated instance is not cleaned on the source host if instance is not defined on the hypervisor" [Medium,In progress] https://launchpad.net/bugs/1724172 - Assigned to Balazs Gibizer (balazs-gibizer)
08:23:51 openstackgerrit Balazs Gibizer proposed openstack/nova master: Reproduce bug 1724172 in the functional test env https://review.openstack.org/512553
08:26:57 lyarwood stephenfin: https://review.openstack.org/#/c/564257/ - would you mind taking a swing at this today?
08:27:19 lyarwood stephenfin: awkward hard reboot regression we could really use backported to queens
08:27:28 lyarwood stephenfin: well, the fix not the bug ;)
08:27:43 stephenfin Will do
08:27:51 lyarwood cheers
08:29:54 stephenfin lyarwood: aaand done. That was easy
08:40:46 openstack Launchpad bug 1766208 in OpenStack Compute (nova) "Lift the restriction on choices for `cpu_model_extra_flags` config attribute" [Undecided,Invalid] - Assigned to Kashyap Chamarthy (kashyapc)
08:40:46 kashyap I know Matt is right "strictly speaking", this isn't a "bug": https://bugs.launchpad.net/nova/+bug/1766208
08:40:57 kashyap But I filed it as a mostly "item tracker"

Earlier   Later