Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-21
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 gibi mriedem: fyi here is a followup bug for the notifications-calling-keystone problem https://bugs.launchpad.net/nova/+bug/1757407
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: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
15:33:47 mriedem because it's going to require loading a bunch of context into my head, including stuff like mixed version compute issues
15:34:15 dansmith mriedem: I don't think it does actually,
15:34:19 gibi mriedem: maybe not even trying to generate the glance url (and call keystone) if the glance/api_server config is not set
15:34:20 dansmith mriedem: it doesn't change what we do over the wire
15:34:21 mriedem gibi: can't really do much there for the legacy ones
15:35:01 mriedem i'm mid-review in sahid's spec review so will have to look in a bit
15:35:09 dansmith ack, thanks
15:35:13 stvnoyes1 dansmith, thanks, I'll take a look at it. I'll have to remember what I did and why.
15:36:58 mriedem everything stvnoyes1 added would have just been conditional logic on the flow for the new attachment ID stuff
15:37:26 mriedem which was fairly mechanical, i.e. 'we used to call initialize_connection to get a new connection_info, now we call attachment_update'
15:37:59 dansmith mriedem: yeah, but the intersection is that this seems to preclude adding old_attachment_id to the migratedata stuff for the v3 attach,
15:38:15 dansmith but also fixes the same issue that old_attachment_id fixes for v3, but for v2
15:38:35 dansmith I mean, that's the assertion
15:39:25 dansmith mdbooth: the bug is basically just the commit message on the patch.. can you add some more detail (logs, traces, etc) to help make it more convincingly problematic?
15:39:52 dansmith mdbooth: because since the old side of all this code has been around for a while, it's legit to question that it's been broken this long, even if it's just for one driver
15:46:42 openstackgerrit Merged openstack/nova master: ironic: stop lying to the RT when ironic is down https://review.openstack.org/545479
15:48:13 openstackgerrit Dan Smith proposed openstack/nova master: Add aggregates list to Destination object https://review.openstack.org/544729
15:48:14 openstackgerrit Dan Smith proposed openstack/nova master: Add request filter functionality to scheduler https://review.openstack.org/544730
15:48:15 openstackgerrit Dan Smith proposed openstack/nova master: Make get_allocation_candidates() honor aggregate restrictions https://review.openstack.org/547990
15:48:16 openstackgerrit Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002
15:48:16 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282
15:50:16 mriedem sahid: question in https://review.openstack.org/#/c/485522/ about how the guest knows if the VFs are trusted
15:52:20 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: Handle EndpointNotFound when building image_ref_url in notifications https://review.openstack.org/554963
15:57:35 mriedem alright now for this live migration funbag
15:58:13 dansmith mriedem: I just threw some more info into the bug from our downstream details
15:58:22 dansmith might be worth reading first, it's short
16:00:26 mriedem alright. i do remember talking about this a bit with mdbooth pre-patch
16:04:34 mriedem oh btw, here i can trade a volume issue during cold migration https://review.openstack.org/#/c/554667/
16:04:39 mriedem much easier
16:05:51 dansmith sure, I'll take that trade
16:06:48 sahid mriedem: good i did not provided any information about that point, do you know that there is a ask for that?
16:06:58 sahid s/good/good question
16:07:29 mriedem sahid: i think the user impact section of the spec says that with this, the user can change the mac address,
16:07:39 mriedem but that happens in the guest (not noted clearly really in that section),
16:07:49 sahid mriedem: yes in the guest
16:07:54 openstackgerrit Surya Seetharaman proposed openstack/nova master: Add CellMappingList.get_by_disabled() query method https://review.openstack.org/550188
16:07:54 openstackgerrit Surya Seetharaman proposed openstack/nova master: Allow scheduling only to enabled cells (Filter Scheduler) https://review.openstack.org/550527
16:07:59 mriedem and i was thinking, how is the guest going to know if it can do this unless there are device tags on that sriov port in the config drive / metadata
16:08:08 mriedem or is there another way?
16:08:50 mriedem we don't model VFs in the metadata that goes into the guest do we?
16:09:07 mriedem i thought it was just block devices and ports
16:10:11 sahid from the guest, instead of trying to change the MAC address, we can't really know if the VF is using trusted or mode
16:10:14 sahid about metadata
16:10:24 sahid i think there were some work around that
16:10:26 sahid i will check
16:10:38 sahid perhaps we could add a new attribute
16:10:59 mriedem i'm looking at nova.network.netutils.get_network_metadata
16:12:27 mriedem seems like the _get_eth_link method would be the place to take information off the vif's port binding profile that has the 'trusted' flag and put that into the entry
16:12:34 artom mriedem, we have a generic 'devices' section in the metadata, we can add stuff to it
16:12:45 artom hyperv already do stuff in there that's not related to device tagging

Earlier   Later