| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-25 | |||
| 19:28:00 | mriedem | https://github.com/openstack/tempest/tree/master/tempest/scenario | |
| 19:28:03 | melwitt | this is the basic functionality scenario test that verifies ssh and config drive https://github.com/openstack/tempest/blob/master/tempest/scenario/test_server_basic_ops.py#L51 | |
| 19:28:09 | mriedem | stuff like test_minimum_basic | |
| 19:28:22 | edmondsw | will look | |
| 19:28:25 | mriedem | test_server_basic_ops | |
| 19:28:25 | edmondsw | esberglu fyi ^ | |
| 19:28:49 | mriedem | for stuff in that requires volumes, we could fix those in tempest if the env doesn't have cinder | |
| 19:29:22 | mriedem | actually that's already handled https://github.com/openstack/tempest/blob/master/tempest/scenario/test_minimum_basic.py#L102 | |
| 19:29:26 | mriedem | @utils.services('compute', 'volume', 'image', 'network') | |
| 19:29:35 | mriedem | so if the volume service isn't enabled in tempest.conf, that test is skipped | |
| 19:29:55 | mriedem | should totally be able to do this though https://github.com/openstack/tempest/blob/master/tempest/scenario/test_server_basic_ops.py#L125 | |
| 19:30:20 | mriedem | create a server with a keypair and config drive, ssh into it, done | |
| 19:33:58 | lbragstad | mriedem: was this the policy+placement patch you were talking about? | |
| 19:33:59 | lbragstad | https://review.openstack.org/#/c/524425/ | |
| 19:34:12 | melwitt | yeah. I'd personally want to hold off on merging the new driver patches until test_server_basic_ops is running in CI | |
| 19:34:27 | melwitt | with ssh validation enabled | |
| 19:36:04 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Snapshot https://review.openstack.org/543023 | |
| 19:36:05 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053 | |
| 19:36:06 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 19:36:13 | edmondsw | melwitt even things that wouldn't have anything to do with that, like snapshot? | |
| 19:37:01 | melwitt | edmondsw: yeah, it seems like verifying the booted instances was missed for whatever reason in the past, should fix that before adding more stuff I would think | |
| 19:37:36 | dansmith | we hold everyone else to that bar, not sure why it should be a surprise, | |
| 19:37:40 | dansmith | nor why it was skipped | |
| 19:37:44 | esberglu | edmondsw: efried: ^ Commit chain is up without vSCSI. You're votes are still on DiskAdapter and Localdisk. Snapshot lost them in a previous manual rebase | |
| 19:37:53 | esberglu | your | |
| 19:37:57 | mriedem | lbragstad: yeah | |
| 19:43:27 | efried | esberglu: snapshot re-reviewed. | |
| 19:55:11 | openstackgerrit | Julia Kreger proposed openstack/nova master: ironic: add instance_uuid before any other spawn activity https://review.openstack.org/563722 | |
| 19:58:11 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: resource requests for nested providers https://review.openstack.org/554529 | |
| 20:00:52 | openstackgerrit | Jay Pipes proposed openstack/nova master: support multiple member_of qparams https://review.openstack.org/561315 | |
| 20:14:03 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use inspect.signature in nova.utils.expects_func_args https://review.openstack.org/564333 | |
| 20:14:04 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use inspect.signature in nova.network.base_api.refresh_cache https://review.openstack.org/564334 | |
| 20:22:12 | mriedem | yikun_: at this point, you should probably just incorporate https://review.openstack.org/#/c/563802/ into https://review.openstack.org/#/c/561819/ and https://review.openstack.org/#/c/557145/ respectively | |
| 20:22:31 | openstackgerrit | Jay Pipes proposed openstack/nova master: support multiple member_of qparams https://review.openstack.org/561315 | |
| 20:22:36 | mriedem | rather than a never-ending series of "address nits" patches at the end | |
| 20:25:05 | openstackgerrit | Merged openstack/nova master: Update docs for [keystone_authtoken] changes since Queens https://review.openstack.org/562812 | |
| 20:25:14 | openstackgerrit | Merged openstack/nova master: Make accept-language tests work with webob 1.8.x https://review.openstack.org/564255 | |
| 20:26:10 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Update docs for [keystone_authtoken] changes since Queens https://review.openstack.org/564336 | |
| 20:27:20 | melwitt | yikun_: seconded, especially the reno update. seems kinda weird to split that up | |
| 20:28:00 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: resource requests for nested providers https://review.openstack.org/554529 | |
| 20:28:31 | efried | mriedem: Is there a way to elevate the getargspec deprecation warning to error in py3, to prove those two patches you just proposed? | |
| 20:30:39 | mriedem | lbragstad: do you know the answer to this? https://review.openstack.org/#/c/524425/1/nova/api/openstack/placement/policies/base.py | |
| 20:31:00 | mriedem | efried: yeah i think so | |
| 20:31:12 | mriedem | https://docs.python.org/2/library/warnings.html#the-warnings-filter | |
| 20:33:00 | mriedem | https://docs.python.org/2/library/warnings.html#warnings.filterwarnings | |
| 20:33:19 | mriedem | like https://github.com/openstack/nova/blob/6d22567b9445e595f4c55627ea784801585a6e41/nova/tests/fixtures.py#L731 | |
| 20:33:46 | mriedem | err the ones below | |
| 20:33:50 | lbragstad | mriedem: i was just digging into that | |
| 20:33:51 | efried | mriedem: Would it be unreasonable to ask you to do that in those? | |
| 20:33:55 | mriedem | efried: i could | |
| 20:34:00 | lbragstad | one sec | |
| 20:34:18 | lbragstad | i want to say the answer is in the NovaKeystoneContext object | |
| 20:34:18 | mriedem | efried: note there is another inspect.getargspec usage in nova/cmd/common.py but it doesn't have any unit testing yet so i haven't tackled that one | |
| 20:34:30 | mriedem | lbragstad: nova.context.RequestContext? | |
| 20:35:17 | mriedem | efried: i'll throw something on top | |
| 20:35:22 | efried | ++ | |
| 20:36:53 | lbragstad | mriedem: first crack at working some documentation into oslo.policy's usage docs for newer services - https://review.openstack.org/#/c/564340/ | |
| 20:37:28 | lbragstad | mriedem: maybe - but nova has a special middleware that sits right behind auth_token | |
| 20:37:47 | lbragstad | it's job is to pull the auth token headers out of the request and generate a context object | |
| 20:38:00 | lbragstad | at least from what i can tell | |
| 20:38:22 | lbragstad | https://github.com/openstack/nova/blob/d124a0ed87cb99a06954ae7f3073cb859aa6808f/etc/nova/api-paste.ini#L80-L81 | |
| 20:38:47 | lbragstad | https://github.com/openstack/nova/blob/d124a0ed87cb99a06954ae7f3073cb859aa6808f/etc/nova/api-paste.ini#L32 | |
| 20:40:00 | mriedem | lbragstad: yeah i see it now | |
| 20:40:18 | mriedem | https://github.com/openstack/nova/blob/d124a0ed87cb99a06954ae7f3073cb859aa6808f/nova/api/auth.py#L72 | |
| 20:41:05 | mriedem | i don't think that has anything to do with rule:admin_api though | |
| 20:44:13 | lbragstad | yeah - digging through it and it doesn't look like it | |
| 20:44:31 | lbragstad | my bad | |
| 20:46:56 | lbragstad | it looks like rule:context_is_admin is really just an alias to role:admin | |
| 20:47:50 | lbragstad | https://github.com/openstack/nova/blob/master/nova/policy.py#L173 | |
| 20:49:31 | lbragstad | which is apparently used in nova's RequestContext https://github.com/openstack/nova/blob/d124a0ed87cb99a06954ae7f3073cb859aa6808f/nova/context.py#L151 | |
| 20:53:36 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add columns for generation to consumers https://review.openstack.org/557958 | |
| 20:53:37 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add the Consumer object https://review.openstack.org/561406 | |
| 20:53:38 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add Consumer objects to Allocations https://review.openstack.org/561407 | |
| 20:53:39 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add a microversion for consumer generation https://review.openstack.org/562000 | |
| 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 | |