Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-11-15
20:38:03 mriedem i think if we care about that, we handle the todo later separately
20:38:20 edleafe ok, I'll add a note there.
20:41:56 openstackgerrit Matt Riedemann proposed openstack/nova master: Add a note about versioned notification samples being per-release https://review.openstack.org/520193
20:42:07 openstackgerrit Matt Riedemann proposed openstack/nova master: Add a note about versioned notification samples being per-release https://review.openstack.org/520193
20:48:48 edleafe mriedem: Not sure what the concern about the refactor is. Would it be better to duplicate the code in this patch, and then add a new patch that does the refactor?
20:49:14 sdague mriedem: +A on https://review.openstack.org/#/c/515213/
20:49:47 mriedem sdague: thanks
20:49:58 mriedem edleafe: well, (1) why is the refactor needed?
20:50:15 mriedem because as i said in my comments, the change, per the commit message, is about returning Selection objects from the driver rather than HostState objects
20:50:17 mriedem but this is doing more
20:51:22 mriedem edleafe: in other words, why is _ensure_sufficient_hosts needed? why move that check from select_destinations? is it because by the time we get to select_destinations to validate, we don't have the correct *type* of object?
20:51:28 mriedem i'm guessing host.updated = None is the issue
20:51:36 mriedem because that is for a HostState object, not a Selection object
20:53:27 mriedem edleafe: if _ensure_sufficient_hosts is needed because of $reason_given_by_me_just_now, then i'm requesting that you split that refactor that introduces _ensure_sufficient_hosts into a patch before https://review.openstack.org/#/c/495854/ in the series
20:53:34 mriedem where we're still dealing with HostState objects
20:53:46 mriedem it would just make review easier to know if we are changing any behavior
20:54:47 edleafe mriedem: ok, I'll add a patch before this one.
20:55:08 mriedem cool, thanks
20:55:18 mriedem i think eventually that also allows us to just merge the select_destinations and _schedule methods in the FilterScheduler
20:55:23 mriedem collapse them
20:55:27 mriedem but - later
20:58:38 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix docstring for GET /os-migrations and related DB API https://review.openstack.org/520200
21:16:11 efried jaypipes http://paste.openstack.org/raw/626443/
21:16:41 jaypipes efried: told ya :) big difference.
21:16:46 efried jaypipes ++
21:35:27 efried jaypipes Is it your vision that update_inventory be mutually exclusive with get_inventory? That is, if a virt driver implements the former, the latter is ignored?
21:36:49 efried jaypipes I had thought that implementing get_inventory caused get_available_resource to be ignored, but it kinda looks like that's not the case?
21:37:27 jaypipes efried: unfortunately, we need to continue supporting get_available_resource() until all resource classes are handled by placement.
21:37:42 jaypipes efried: it was my vision that update_inventory() would override get_inventory() when present, yes.
21:37:52 efried jaypipes Okay.
21:41:28 openstackgerrit Matt Riedemann proposed openstack/nova master: Add migration_get_by_uuid in db api. https://review.openstack.org/511421
21:41:29 openstackgerrit Matt Riedemann proposed openstack/nova master: Add migration db and object pagination support. https://review.openstack.org/514904
21:41:29 openstackgerrit Matt Riedemann proposed openstack/nova master: Implement query param schema for migration index https://review.openstack.org/518644
21:41:30 openstackgerrit Matt Riedemann proposed openstack/nova master: Extract SortContext/SortWrapper of instance_list into utils https://review.openstack.org/518871
21:41:30 openstackgerrit Matt Riedemann proposed openstack/nova master: Add cross cell sort support for get_migrations https://review.openstack.org/517273
21:41:31 openstackgerrit Matt Riedemann proposed openstack/nova master: Add pagination and Changes-since filter support for os-migrations. https://review.openstack.org/330406
22:08:34 openstackgerrit Merged openstack/nova master: Trivial grammar fix https://review.openstack.org/517283
22:10:40 openstackgerrit Matt Riedemann proposed openstack/nova master: Add microversion to allow setting flavor description https://review.openstack.org/515213
22:10:59 mriedem sdague: ^ i rebased to kick it into the gate, it didn't go through for some reason
22:17:23 openstackgerrit Eric Berglund proposed openstack/nova master: WIP(5): PowerVM driver: ovs vif https://review.openstack.org/422512
22:54:55 eandersson Is there a reason why this flag is always set to False? https://github.com/openstack/nova/blob/mitaka-eol/nova/virt/driver.py#L1249
22:56:14 eandersson In certain scenarios when nova-compute fails to schedule it does not clean up the network ports
22:56:35 eandersson e.g. when hitting > qemu-kvm: cannot set up guest memory 'pc.ram': Cannot allocate memory
22:57:10 mriedem eandersson: it's apparently specific to baremetal
22:57:15 mriedem in how it's used anyway
22:58:19 mriedem otherwise i think ports created for an instance will go with the instance and be bound on the next host
22:58:34 eandersson https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1880
22:58:50 eandersson It's used in this code path
22:59:10 mriedem yeah i'm looking at it locally
22:59:13 eandersson and basically if this function is called self._cleanup_allocated_networks
22:59:17 eandersson it will clean it up properly
22:59:33 mriedem what do you actually need cleaned up before the reschedule?
22:59:48 mriedem the ports should not be bound to that host at that point
22:59:56 eandersson Well the problem we are hitting is that each time it gets re-scheduled, a new ip is assigned
23:00:03 mriedem is a new port created?
23:00:08 eandersson Yea
23:00:15 mriedem ok, that's a problem, and i think an old one
23:00:29 mriedem i'm sure i could find some bugs if i dug for a bit
23:00:49 eandersson I am reproducing it with this http://paste.openstack.org/show/626450/
23:00:55 mriedem https://bugs.launchpad.net/nova/+bug/1703540
23:00:55 openstack Launchpad bug 1703540 in OpenStack Compute (nova) "Reschedule with libvirt exception leaves dangling neutron ports" [High,Triaged]
23:01:08 mriedem https://bugs.launchpad.net/nova/+bug/1531022
23:01:09 openstack Launchpad bug 1531022 in OpenStack Compute (nova) "libvirt driver doesn't cleanup the tap interface on vm re-schedule" [Low,Confirmed]
23:01:24 eandersson oh not sure why I didn't find that one
23:01:59 mriedem yeah see comment #3 on https://bugs.launchpad.net/nova/+bug/1703540
23:02:00 openstack Launchpad bug 1703540 in OpenStack Compute (nova) "Reschedule with libvirt exception leaves dangling neutron ports" [High,Triaged]
23:02:23 eandersson Yep - perfect
23:02:31 eandersson that is the exact same conclusion I came to
23:02:37 mriedem it would be as easy as calling cleanup_instance_network_on_host to remove any ports we created
23:03:03 mriedem however, doing that for neutron would basically be the same as for _cleanup_allocated_networks i think
23:03:15 mriedem because nova has to be smart about not deleting ports that the user supplied vs ones that nova created
23:03:44 eandersson Is there a reason why we can't just call _cleanup_allocated_networks ?
23:03:53 eandersson I tested that out in my lab and it worked 100% of the time
23:04:38 eandersson but obviously my test case is rather limited
23:05:16 mriedem honestly i'm not sure, and i worry that doing that will break something else, which is probably why we've never just done that
23:05:24 mriedem there was this patch for a related bug long ago https://review.openstack.org/#/c/367316/
23:06:37 mriedem oh that was a backport
23:07:11 mriedem ok so https://review.openstack.org/#/c/243477/ is a different fix,
23:07:29 mriedem ^ is if all reschedules fail and we've run out of retries i think, so the instance is set to ERROR
23:07:32 mriedem which is different than,
23:07:37 mriedem host A fails, host B works
23:07:41 mriedem you'd have 2 ports
23:07:48 openstackgerrit Michael Still proposed openstack/nova master: Convert ext filesystem resizes to privsep. https://review.openstack.org/517516
23:07:49 openstackgerrit Michael Still proposed openstack/nova master: Move flushing block devices to privsep. https://review.openstack.org/519010
23:07:49 openstackgerrit Michael Still proposed openstack/nova master: Start moving users of parted to privsep. https://review.openstack.org/519011
23:07:50 openstackgerrit Michael Still proposed openstack/nova master: Move remaining uses of parted to privsep. https://review.openstack.org/519483
23:07:50 openstackgerrit Michael Still proposed openstack/nova master: Convert users of tune2fs to privsep. https://review.openstack.org/519484
23:08:20 eandersson I see
23:10:53 mriedem i'm also not entirely sure if _cleanup_allocated_networks will do the entire job since it doesn't explicitly unplug vifs via the virt driver,
23:11:08 mriedem i'm not sure if deleting the port will take care of that out of band?
23:11:28 mriedem https://bugs.launchpad.net/nova/+bug/1531022/comments/2
23:11:28 openstack Launchpad bug 1531022 in OpenStack Compute (nova) "libvirt driver doesn't cleanup the tap interface on vm re-schedule" [Low,Confirmed]
23:12:21 efried jaypipes More sanity check: In order to pass the entire provider tree to update_inventory, I'm going to need to *get* the entire tree. Which entails a new method in report client that calls GET with the new ?tree= deal, then builds up the ProviderTree object accordingly. And then actually returns that guy, which is currently a private _provider_tree attribute and not actually returned anywhere yet.
23:12:27 eandersson I don't see the tap at least on the compute I tested it on
23:12:52 eandersson but this is mitaka, who knows how it would work on other versions of Openstack
23:13:08 mriedem well, does it maybe depend on the type of interface?
23:13:12 mriedem vif type i mean
23:13:23 efried jaypipes Then when I get it back from update_inventory, the driver may have added children and whatnot, so I'm going to have to walk the tree to rearrange the placement db accordingly.
23:14:01 mriedem eandersson: here is another one https://review.openstack.org/#/c/335788
23:14:13 efried jaypipes And also call _normalize_inventory_from_cn_obj on every provider in the tree, in case they decided to shove their VCPU/MEMORY_MB/DISK_GB in children instead of leaving them in the root.

Earlier   Later