Earlier  
Posted Nick Remark
#openstack-nova - 2018-03-19
20:52:58 sean-k-mooney2 mriedem: do we do the vif plugging in pre_livemigrate today or is it down after we bind the port on the destination after livemigration completes
20:53:34 mriedem the dest host plugs the vif in pre_live_migration today
20:53:55 mriedem source rpc calls to dest pre_live_migration, and then once that rpc call returns, the source starts live migrating the guest in the hypervisor
20:54:03 sean-k-mooney2 ok so ya we should be seeing the same bevavior.
20:54:09 mriedem this was part of the thing where the source host needs to wait for the vif plugged event from the dest
20:54:23 mriedem before it starts transferring the guest
20:54:30 mriedem i haven't coded that part up yet
20:55:15 mriedem for all i know, given https://review.openstack.org/#/c/553035/ - can we even reliably wait for vif-plugged on the source if the host binding hasn't changed?
20:55:28 sean-k-mooney2 when you call self.network_api.migrate_instance_finish(context, instance,...) does that activate the port binding on the dest
20:55:30 mriedem or will opendaylight never send a vif-plugged event in that case?
20:55:50 mriedem sean-k-mooney2: it switches the binding host_id yeah, sec
20:56:18 mriedem https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L2577
20:56:27 mriedem in this case, that host variable is the dest host
20:56:48 mriedem that's what we see here http://logs.openstack.org/71/551371/6/check/legacy-tempest-dsvm-multinode-live-migration/4d466b2/logs/subnode-2/screen-n-cpu.txt.gz#_Mar_19_14_25_06_838681
20:56:52 mriedem on the source host
20:57:01 sean-k-mooney2 mriedem: im not sure if odl will remit the event but i dont think that would be an unreasonable expectation.
20:57:40 mriedem sean-k-mooney2: from what i remember of the discussion leading up to https://review.openstack.org/#/c/553035/ with mnaser, ODL will only emit events for host binding changes, not vif unplug/plug
20:58:24 mriedem which really kind of kills us as the consumer of this workflow...
20:58:45 sean-k-mooney2 yes and when we activate the binding for the dest that should be considered a binding change as we update the host_id in the port bindings_details field on the port
21:00:05 mriedem yeah, but the plan was to not start migrating the guest in the hypervisor until the source got the event that the plug, initiated from the dest, is done
21:00:45 _ix Good afternoon, folks. Can anyone explain the pipeline from the fine work that's going into nova and the end repositories at say http://mirror.centos.org/centos/7/cloud/x86_64/openstack-pike/
21:01:14 sean-k-mooney2 mriedem: right so we might have to start the migrate on a timeout and have the event short circute it, instead of waiting
21:01:15 mriedem really kind of need an admin-only field on the port to tell clients, like nova, if we can expect vif plug events or not
21:02:03 _ix I'm only seeing latest as 16.0.3 in that repo -- is there a different repository that you mgith be able to recommend for 16.1.0 ?
21:02:10 mriedem sean-k-mooney2: we could, but then anyone using ODL is going to wait up to 5 minutes for an event that's not going to come
21:02:50 mriedem _ix: you could try asking in #openstack-rpm-packaging
21:03:03 sean-k-mooney2 mriedem: ill double check the odl code. ill be back in the office tomorrow so i can try and set up an odl environment
21:03:13 mriedem _ix: or maybe #rdo
21:03:34 mriedem sean-k-mooney2: ok for now i'm just going to neuter this lifecycle event callback code that calls migrate_instance_finish
21:05:02 sean-k-mooney2 well we can still activate the dest binding when we get the hyperviors event that it just paused the source and is about to unpause the dest right
21:05:20 sean-k-mooney2 we just dont wait for the vif plugged event
21:05:52 mriedem two separate issues,
21:06:11 mriedem i'm going to neuter the first right now because it's causing this vif unbound explosion in _post_live_migrate
21:06:32 mriedem the latter issue is not something we're hitting right now, but was in the plan as part of this spec
21:06:52 sean-k-mooney2 ah ok
21:17:48 openstackgerrit Merged openstack/nova master: Move placement exceptions into the placement package https://review.openstack.org/549862
21:17:51 _ix mriedem: Thanks for the tips.
21:27:28 jaypipes phew, my brain is melting today...
21:38:28 cdent was it the videophone jaypipes ?
21:40:14 edleafe efried: ugh - I had all those changes, and then rebased poorly after the original member_of patch merged. Fixing...
21:41:01 efried edleafe: Here to keep you honest. I feel your rebase pain, homey.
21:42:02 edleafe efried: heh, trying to keep things straight while half-paying attention to meetings
21:45:58 cdent efried: "I have looked at this." beyond that and the typos, do you think I'm on the right track?
21:46:30 efried cdent: The unedited version was something like, "I have looked at this. I have no idea what I'm looking at, so I am abstaining from voting."
21:46:48 cdent ah, that makes a bit more sense
21:47:19 efried cdent: "...and the time it would take me to figure out what I'm looking at doesn't fit in my current budget (still ploughing through vacation backlog)."
21:47:42 cdent if/when you surface from that and you want a tour, let me know
21:48:11 cdent thanks for looking in any case, I'm always glad to have your proofing
22:08:13 edleafe efried: let
22:08:17 edleafe oops
22:08:23 openstackgerrit Ed Leafe proposed openstack/nova master: Address issues raised in adding member_of to GET /a-c https://review.openstack.org/554357
22:08:32 edleafe efried: let's try this again ^^
22:08:55 efried ack
22:09:40 jaypipes cdent: nah, trying to fix the nested providers alloc candidates stuff
22:19:08 efried mriedem: 1.19 was for bp placement-aggregate-generation. Gerrit likes to overwrite the topic for all patches when submitting a series, which is how 1.19 ended up in the whiteboard for https://blueprints.launchpad.net/nova/+spec/generation-from-create-provider. FTFY.
22:22:38 efried mriedem: You can mark https://blueprints.launchpad.net/nova/+spec/placement-aggregate-generation done too.
22:22:41 mriedem melwitt: totally random but i was just doing some blame game and came across https://review.openstack.org/#/c/377093/1/nova/rpc.py
22:22:55 mriedem melwitt: ever noticed that RequestContext(overwrite) kwarg is not used at all in that patch?
22:23:11 mriedem https://review.openstack.org/#/c/377093/1/nova/context.py@72
22:24:19 mriedem efried: done
22:24:25 efried thx
22:25:31 melwitt mriedem: yeah, it's used in the base context class from oslo https://github.com/openstack/oslo.context/blob/a8d86df/oslo_context/context.py#L225
22:26:22 mriedem oh i see
22:26:41 melwitt it's kind of unclear though, being lumped into **kwargs like that
22:26:47 mriedem have been trying to figure out wtf periodic tasks are running with a request id which is the same request id i'm tracking for an instance create operation
22:27:41 melwitt ah, okay. I'd like to know how that happens too
22:27:58 mriedem well, when the periodics run, they call get_admin_context
22:28:05 mriedem which doesn't overwrite the thread local context
22:28:16 mriedem so i guess that's why
22:28:20 melwitt oh, so they just get whatever was there. yeah
22:28:38 mriedem https://github.com/openstack/nova/blob/master/nova/service.py#L295
22:28:56 mriedem which are run from this thread group timer https://github.com/openstack/nova/blob/master/nova/service.py#L213
22:29:19 mriedem but it makes tracing a failed operation for a specific instance really frustrating if everything else picks up the thread local version
22:29:23 melwitt right
22:30:14 melwitt I have this old patch up around generating an admin context up front and storing it on the service for use in periodic tasks (to help us get to configuring CellDatabases differently). I'm not sure if it would do any better than the current situation though https://review.openstack.org/#/c/524306
22:30:45 melwitt like, I'm not sure if that initial get_admin_context in the service creation might also get tied to some unhelpful request id
22:31:09 melwitt er, service.start() I mean
22:32:22 openstackgerrit Matt Riedemann proposed openstack/nova master: Add VIFMigrateData object for live migration https://review.openstack.org/515423
22:32:23 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: libvirt: use dest host vif migrate details for live migration https://review.openstack.org/551370
22:32:23 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Port binding based on events during live migration https://review.openstack.org/434870
22:32:24 openstackgerrit Matt Riedemann proposed openstack/nova master: Add "delete_port_binding" network API method https://review.openstack.org/552170
22:32:24 openstackgerrit Matt Riedemann proposed openstack/nova master: compute: use port binding extended API during live migration https://review.openstack.org/551371
22:32:25 openstackgerrit Matt Riedemann proposed openstack/nova master: conductor: use port binding extended API in during live migrate https://review.openstack.org/522537
22:32:25 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Turn on new port binding extended live migrate flow https://review.openstack.org/552173
22:33:36 mriedem it shouldn't if we're not calling get_admin_context() every time the periodic runs
22:34:36 mriedem i was going to try setting the request_id='req-%s' % self.service_ref.uuid so we'd have a predictable request id in periodics, but that would just be ignored since get_admin_context uses overwrite=False
22:35:08 melwitt okay, then that patch might help then. it got stalled on me not providing a reliable poison fixture for making sure nothing in periodics tries to get_admin_context on their own. and gibi proposed something better around that. but I haven't gotten back to it
22:49:14 dansmith mriedem: I'm still not sure how that makes sense,
22:49:25 dansmith the periodics are their own (eventlet) thread, they should have their own TLS
22:49:53 dansmith not sure why they shouldn't be able to set their TLS there
22:50:17 dansmith I'd guess that overwrite=False is actually for the case where some user operation needs an admin context to do something specific, and you don't want to leave them with admin when you're done
22:50:30 dansmith but periodics probably *should* overwrite TLS in that case
22:51:16 dansmith I would also expect TLS to be reset on each spawn/switch of a thread, otherwise you leak things between requests until you set TLS _to_ something
22:51:38 mriedem i assume overwrite=False was added to get_admin_context() for a wholly different reason than why we're using get_admin_context() for periodics
22:52:04 dansmith that's what I'm saing
22:52:16 dansmith for a localized "okay, do this thing for the user as admin"
22:52:23 dansmith looking up a fixedip before assignment, etc
22:52:34 dansmith that's why I'm saying we probably want overwrite=True for periodics
22:53:06 dansmith that kinda goes against the original supposition of melwitt's patch to client RPC though
22:53:11 melwitt here's the original bug that led to using overwrite=False https://bugs.launchpad.net/nova/+bug/1627838

Earlier   Later