| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-21 | |||
| 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: WIP: Honor availability_zone hint via placement https://review.openstack.org/546282 | |
| 15:48:16 | openstackgerrit | Dan Smith proposed openstack/nova master: Add require_tenant_aggregate request filter https://review.openstack.org/545002 | |
| 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: Allow scheduling only to enabled cells (Filter Scheduler) https://review.openstack.org/550527 | |
| 16:07:54 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Add CellMappingList.get_by_disabled() query method https://review.openstack.org/550188 | |
| 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 | |
| 16:13:33 | mriedem | this is specific to network data though | |
| 16:13:59 | mriedem | idk what's the best solution here, i'm not an expert on the metadata service or how a guest would consume trusted SRIOV ports in the guest, | |
| 16:14:15 | mriedem | but it's obviously something we should think about how that gets modeled so the guest application can actually leverage the thing | |
| 16:14:44 | artom | mriedem, wait, we're talking about trusted VFs? | |
| 16:15:22 | artom | What's wrong with just having an interface: {mac: blah, trusted:true} blob in there. | |
| 16:15:22 | mriedem | yes | |
| 16:15:25 | artom | ? | |
| 16:15:35 | artom | The exact format is obviously to be worked out :) | |
| 16:15:36 | mriedem | hold dear caller | |
| 16:16:28 | mriedem | artom: https://review.openstack.org/#/c/485522/ | |
| 16:16:33 | mriedem | questions/comments are in there | |
| 16:16:48 | diga | jaypipes: Hi | |
| 16:17:10 | diga | jaypipes: assigned this bug - https://bugs.launchpad.net/nova/+bug/1751692 to me | |
| 16:17:11 | openstack | Launchpad bug 1751692 in OpenStack Compute (nova) "os_region_name an unnecessary required option for placement " [Low,Triaged] - Assigned to Digambar (digambarpatil15) | |
| 16:17:18 | artom | mriedem, aha, thanks - I keep being dragged off for downstream stuff, but I've already added myself in there, will try to take a look | |