| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-21 | |||
| 14:41:02 | Kevin_Zheng | the req_id didn't match | |
| 14:41:11 | Kevin_Zheng | which I did replaced | |
| 14:43:06 | mriedem | is an admin context getting generated somewhere? | |
| 14:43:22 | mriedem | context.get_admin_context() will return a unique request id | |
| 14:44:51 | Kevin_Zheng | I will check | |
| 14:47:05 | openstackgerrit | Merged openstack/nova master: trivialfix: cleanup _pack_instance_onto_cores() https://review.openstack.org/538698 | |
| 14:47:39 | openstackgerrit | Merged openstack/nova master: Handle EndpointNotFound when building image_ref_url in notifications https://review.openstack.org/554703 | |
| 14:47:52 | openstackgerrit | Merged openstack/nova stable/queens: Only attempt a rebuild claim for an evacuation to a new host https://review.openstack.org/550545 | |
| 14:48:12 | openstackgerrit | Merged openstack/nova stable/queens: Unmap compute nodes when deleting host mappings in delete cell operation https://review.openstack.org/553496 | |
| 14:48:42 | jaypipes | bauzas: cfriesen is in EST timezone I think? | |
| 14:48:47 | openstackgerrit | Merged openstack/nova stable/pike: Detach volumes when VM creation fails https://review.openstack.org/544143 | |
| 14:49:00 | bauzas | jaypipes: living in Alberta IIRC | |
| 14:49:16 | jaypipes | ah.. I thought it was Ottawa | |
| 14:53:56 | mriedem | dansmith: thoughts on a better name for this thing in tssurya's disabled cells series? https://review.openstack.org/#/c/550188/12/nova/objects/cell_mapping.py@165 | |
| 14:54:02 | Kevin_Zheng | mriedem thanks for the comment and advise, I will have to check the details tomorrow, it is late here :) | |
| 14:54:11 | mriedem | Kevin_Zheng: np, ttyl | |
| 14:54:27 | mriedem | tssurya: also, i wonder if it would be better if the 'disabled' param doesn't have a default | |
| 14:54:48 | mriedem | since 'enabled_or_disabled' is further confused by the fact it has a default behavior | |
| 14:55:02 | tssurya | yea I was waiting for inputs regarding the name for that function | |
| 14:55:34 | openstackgerrit | Silvan Kaiser proposed openstack/nova master: Exec systemd-run with privileges in Quobyte driver https://review.openstack.org/554195 | |
| 14:55:42 | dansmith | mriedem: commented | |
| 14:56:25 | mriedem | i'm cool with get_by_disabled, but don't default the 'disabled' param? | |
| 14:56:32 | mriedem | so caller has to know what they are asking for | |
| 14:56:41 | dansmith | yep | |
| 14:57:08 | mriedem | ok wfm | |
| 14:57:12 | tssurya | wiat, so get_by_disabled() will give enabled by default right ? | |
| 14:57:16 | mriedem | no | |
| 14:57:18 | mriedem | no default | |
| 14:57:21 | mriedem | no kwarg | |
| 14:57:22 | stephenfin | Is there another stable core that could take a look at this, please? https://review.openstack.org/#/c/550079/ | |
| 14:57:29 | tssurya | ah so the user has to pass a value | |
| 14:57:34 | mriedem | stephenfin: i can | |
| 14:57:37 | mriedem | tssurya: yeah | |
| 14:57:41 | tssurya | it becomes mandatory, got it | |
| 14:57:48 | tssurya | works for me as well | |
| 14:57:55 | tssurya | thanks | |
| 14:59:18 | stephenfin | mriedem: Thank you | |
| 15:00:08 | mriedem | stephenfin: do you want to propose a release for os-vif on master? | |
| 15:00:14 | mriedem | if this is high severity | |
| 15:00:33 | openstackgerrit | Dan Smith proposed openstack/nova master: Add aggregates list to Destination object https://review.openstack.org/544729 | |
| 15:00:34 | openstackgerrit | Dan Smith proposed openstack/nova master: Make get_allocation_candidates() honor aggregate restrictions https://review.openstack.org/547990 | |
| 15:00:34 | openstackgerrit | Dan Smith proposed openstack/nova master: Add request filter functionality to scheduler https://review.openstack.org/544730 | |
| 15:00:35 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282 | |
| 15:00:35 | openstackgerrit | Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002 | |
| 15:04:39 | mriedem | stephenfin: what is the minimum version that --may-exist exists in ovs-vsctl? | |
| 15:10:12 | mriedem | looks like forever ago https://github.com/openvswitch/ovs/commit/bb1c67c813c9bd80c2bd9acf2bf5158b48841c61 | |
| 15:12:13 | cfriesen | bauzas: you wanted to set up a meeting? I'm in CST timezone...it's 9:12. | |
| 15:12:39 | bauzas | cfriesen: just discussing about NUMA topology | |
| 15:12:53 | bauzas | cfriesen: but jaypipes asked for tomorrow morning EST | |
| 15:13:16 | cfriesen | should be doable | |
| 15:13:27 | cfriesen | jaypipes: my team is in Ottawa, I'm in Saskatchewan | |
| 15:16:09 | mriedem | sean-k-mooney: are you ok with this on stable/queens? https://review.openstack.org/#/c/554917/ | |
| 15:16:41 | gibi | Kevin_Zheng: sorry I haven't had time yet to think about your request_id functional test problem but I did not forget it | |
| 15:18:01 | openstack | Launchpad bug 1757407 in OpenStack Compute (nova) "Notification sending sometimes hits the keystone API to get glance endpoints" [Undecided,New] | |
| 15:18:01 | gibi | mriedem: fyi here is a followup bug for the notifications-calling-keystone problem https://bugs.launchpad.net/nova/+bug/1757407 | |
| 15:18:35 | gibi | mriedem: there is a case where we hit keystone even if only versioned notifications are configured to be emitted | |
| 15:18:48 | Kevin_Zheng | gibi: np I was also busy these days so I didn’t dig deeper, I will try to find out what’s going on tomorrow:) | |
| 15:19:19 | stephenfin | mriedem: Yup, it's there since forever. There was an issue with it but that was resolved in...OVS 2.5, iirc | |
| 15:19:40 | stephenfin | and it wasn't a significant issue. Could only be reproduced under very specific circumstances | |
| 15:21:18 | mriedem | gibi: yeah so we could optimize to not even do that lookup if only using versioned notifications, | |
| 15:21:39 | mriedem | also, efried said the glance endpoint / service catalog information should be cached in ksa, so we shouldn't be hitting the keystone API every time, only the first time, | |
| 15:21:59 | stephenfin | mriedem: Also, I can propose a fix, yup | |
| 15:22:08 | mriedem | but it's curious that we could create a server (which would fetch the image on the compute) and then the periodic (without a token) would have problems stopping it | |
| 15:22:31 | mriedem | unless you did something like had (1) cached images on the compute or (2) restarted nova-compute in between to invalidate the ksa cache | |
| 15:22:45 | openstackgerrit | Merged openstack/os-vif stable/queens: ovs: do not delete port if already exists https://review.openstack.org/550079 | |
| 15:22:50 | mriedem | unless the cache has a timer on it? or is somehow otherwise request-specific | |
| 15:23:12 | gibi | mriedem: I can try to create a functional test for this | |
| 15:23:25 | sahid | jaypipes, mriedem, anychance to have you ack this ? https://review.openstack.org/#/c/485522/ | |
| 15:23:41 | mriedem | sahid: i'll look at it again today | |
| 15:24:04 | mriedem | s/today/now | |
| 15:24:30 | sahid | cool thanks | |
| 15:24:53 | efried | mriedem, gibi: The caching would be specific to the context. So you would be hitting the endpoint (to do version discovery) once per unique context (as opposed to just the first time overall). | |
| 15:25:09 | efried | ...I think. | |
| 15:25:19 | mriedem | ah ok | |
| 15:25:22 | mriedem | well that makes sense then | |
| 15:25:33 | gibi | efried, mriedem: a periodic task runs with the same context every time, i guess | |
| 15:26:03 | mriedem | gibi: nope | |
| 15:26:26 | mriedem | gibi: periodic tasks actually run with the last context stored in the local thread, | |
| 15:26:41 | mriedem | something like that, but that's why we see request IDs for user contexts get mixed up with periodic tasks in the logs | |
| 15:26:44 | mriedem | and it's totally confusing | |
| 15:26:45 | gibi | mriedem: I thought about this https://review.openstack.org/#/c/524306 | |
| 15:27:00 | mriedem | yes we were talking about that the otherday | |
| 15:27:41 | gibi | mriedem: so that patch will make sure that the periodic task runs with the same context every time | |
| 15:28:10 | mriedem | for the lifetime of that service process yes i think so | |
| 15:28:16 | gibi | mriedem: which will limit the performance impact of the notifications calling keystone | |
| 15:28:30 | mriedem | well, the notifications can't call keystone anyway | |
| 15:28:31 | gibi | mriedem: due to the cache in ksa | |
| 15:28:32 | mriedem | they don't have a token | |
| 15:29:01 | gibi | mriedem: OK, I'm confused. :) | |
| 15:29:12 | mriedem | we realized that "ctxt = context.get_admin_context()" has this overwrite=False flag: https://github.com/openstack/nova/blob/master/nova/context.py#L290 | |
| 15:29:20 | mriedem | which is used in the parent class in oslo.context | |
| 15:29:43 | mriedem | https://github.com/openstack/oslo.context/blob/master/oslo_context/context.py#L225 | |
| 15:31:18 | gibi | mriedem: so if the first call to keystone from the periodic task fails then there is nothing to cache so the next call will also go to keystone and fail again | |
| 15:31:23 | mriedem | from what i can tell, that's why periodic tasks re-use the request id from the local thread store | |
| 15:31:32 | mriedem | gibi: i think so | |
| 15:31:57 | dansmith | mriedem: stvnoyes1: this patch is changing code you wrote for the new attachment workflow, but makes it much simpler.. what am I missing? https://review.openstack.org/#/c/551302 | |
| 15:32:04 | gibi | mriedem: then definitely need remove the keystone call from the notification codepath | |
| 15:32:40 | gibi | mriedem: what do you think can we do someting with the legacy notifications top of what you already did by catching the exception? | |
| 15:32:57 | gibi | mriedem: to avoid the keystone call | |
| 15:33:29 | mriedem | dansmith: i haven't dug into that one in detail yet | |