| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-15 | |||
| 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. | |
| 23:14:52 | mriedem | "If I understand the code correctly, the intention of not deleting the port when rescheduling is that it could be reused on the second compute node. But that reuse does not seem to happen, instead nova allocates another port, leaving the first one pending." | |
| 23:15:11 | mriedem | eandersson: it could also totally be some legacy nova-network thing | |
| 23:15:20 | efried | jaypipes That first thing sort of a tree-ified version of _ensure_resource_provider | |
| 23:15:28 | mriedem | like maybe we didn't deallocate networks on reschedule if you were using nova-net? i'm not sure | |
| 23:17:32 | eandersson | Yea - I have done a lot of testing, but I am hesitant to change this without knowing all potential side-effects (even if only for internal usage) | |
| 23:17:45 | eandersson | but it being a legacy thing would make a lot of sense | |
| 23:20:50 | mriedem | eandersson: ok i'm going to duplicate some of the bugs to just this one https://bugs.launchpad.net/nova/+bug/1597596 | |
| 23:20:50 | openstack | Launchpad bug 1597596 in OpenStack Compute (nova) "network not always cleaned up when spawning VMs" [Medium,Confirmed] | |
| 23:20:56 | mriedem | which had a patch and did a lot of the same investigation | |
| 23:24:55 | mriedem | eandersson: so https://review.openstack.org/#/c/335788/ isn't going to fly, because it's totally relying on 2 out of tree scheduler hints :/ | |
| 23:25:45 | mriedem | eandersson: part of me just wants to add an "or utils.is_neutron()" to that conditional check on reschedule | |
| 23:25:53 | mriedem | so if there is a legacy case for nova-net, it's not affected here | |
| 23:26:04 | mriedem | and we'll just always remove ports that nova created on reschedule | |
| 23:26:15 | mriedem | dansmith: do you recall any reasons why we don't just always remove ports we created when we reschedule? | |
| 23:26:39 | eandersson | Yea - I don't like 335788 | |
| 23:26:47 | eandersson | but utils.is_neutron would be awesome for this | |
| 23:27:16 | eandersson | that way if there really is some nova-net edge case we would be covered | |
| 23:27:25 | dansmith | um, I don't remember the exact reason, but I think there is one... like setup time or resources for certain backends maybe? | |
| 23:27:52 | mriedem | the bug reports i've been digging through seem to indicate that certain ML2 backends are cool with us not deallocating ports between reschedules | |
| 23:28:00 | mriedem | however, i think from the nova side, | |
| 23:28:09 | mriedem | we're still creating a port each time we call build_and_run_instance | |
| 23:28:20 | mriedem | so even if the underlying network is tying those together somehow, or ignoring them, | |
| 23:28:25 | mriedem | we're at worst eating up port quota for the project | |
| 23:28:28 | mriedem | for ports that we don't use | |
| 23:28:32 | mriedem | s/use/bind/ | |
| 23:28:53 | dansmith | tbh, it's been a long time since I looked at that path, probably before the great compute/conductor split for build | |
| 23:29:06 | mriedem | yeah, so, | |
| 23:29:20 | mriedem | in the interest of our "damn the torpedos" pledge from the ptg | |
| 23:29:23 | mriedem | and just break everything | |
| 23:29:37 | mriedem | i say we add an "is_neutron" check in this case and just always deallocate before reschedluing | |
| 23:29:46 | mriedem | if that breaks some unicorn ML2 backend, let them speak up | |
| 23:29:47 | dansmith | we delete ones we create unceremoniously on delete now? | |
| 23:30:03 | dansmith | and what about offload? | |
| 23:30:04 | mriedem | when the instance is deleted yes we cleanup and delete anything nova created | |
| 23:30:34 | gmann | alex_xu: hi, returned back to tokyo today | |
| 23:30:36 | mriedem | well, as you can guess, | |
| 23:30:44 | mriedem | dansmith: shelve offload won't delete or cleanup the ports at all, | |
| 23:30:52 | mriedem | it only unplugs the vifs via driver.destroy() | |
| 23:31:06 | mriedem | the actual port will still show as being bound to the original host, | |
| 23:31:15 | dansmith | and unshelve can continue using those ports? | |
| 23:31:15 | mriedem | until we unshelve the instance and bind it to a new host | |
| 23:31:23 | mriedem | i assume so... | |
| 23:31:40 | dansmith | I guess I would expect reschedule to do the same thing.. unbind/rebind | |
| 23:31:48 | mriedem | i only have a single node devstack atm so can't really test unshelve to another host and ssh into the guest after it's unshelved | |
| 23:31:54 | dansmith | but using shelve as the poster child is probably a weaker argument than delete | |
| 23:32:14 | mriedem | yeah, anything to do with ports and volumes + shelve is a roll of the dice | |
| 23:32:25 | mriedem | given we didn't actually terminate connetions for the last 4 years for volumes on unshelve | |
| 23:32:25 | dansmith | yeah | |
| 23:32:41 | dansmith | well, doing the same thing as delete is defensible so if that's the plan it seems reasonable | |
| 23:32:52 | mriedem | alright | |
| 23:33:12 | mriedem | i'll see if can wordsmith a reasonable commit message | |
| 23:33:24 | mriedem | because there is also prior art in this area which always confuses me | |
| 23:35:11 | gmann | yikun: re: on https://review.openstack.org/#/c/518644/11 | |
| 23:36:04 | gmann | yikun: all param in query are as string so we use type as string, if any param is integer then we can use non_integer or positive_integer schema | |
| 23:37:13 | gmann | yikun: and on additionalProperties=True we kept that for backward compatibility but at validation layer all the additional properties will be ignored and stripout | |
| 23:38:17 | gmann | yikun: if you are doing migration pagination with microversion then you can make it False. anyways i will check the patch series today | |
| 23:38:39 | mriedem | gmann: i updated that one today | |
| 23:38:47 | mriedem | the schema thing anyway | |
| 23:39:04 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: Microversion 2.54 - Enable reset keypair while rebuild https://review.openstack.org/519572 | |
| 23:39:35 | gmann | mriedem: it is with version bump ? | |
| 23:39:51 | mriedem | no | |
| 23:39:55 | mriedem | it's all backward compatible | |
| 23:39:59 | mriedem | because additionalProperties=True | |
| 23:40:24 | mriedem | i removed cell_name from the list of query parameters in the schema because it's not used anywhere in the actual nova code, | |
| 23:40:29 | mriedem | novaclient sends it, but shouldn't | |
| 23:40:38 | mriedem | so i'm just treating cell_name like additionalProperties | |
| 23:40:49 | mriedem | in a far flung future microversion we can change all of that | |
| 23:42:06 | gmann | mriedem: i see. | |
| 23:42:12 | gmann | got confuse with https://review.openstack.org/#/q/topic:bp/add-pagination-and-change-since-for-migration-list+(status:open+OR+status:merged) | |
| 23:42:34 | gmann | that patch is just putting schema for existing filters. | |
| 23:42:38 | mriedem | yes, | |
| 23:42:46 | gmann | patch topic was new BP | |
| 23:42:47 | mriedem | it was part of https://review.openstack.org/#/c/330406/ originally and i asked that he split it out | |
| 23:42:53 | gmann | ok | |
| 23:43:33 | mriedem | ah yeah https://review.openstack.org/#/c/330406/58/nova/api/openstack/compute/schemas/migrations.py@47 | |
| 23:43:43 | mriedem | so in 2.55 he'd now restrict to the list of known parameters | |
| 23:44:01 | gmann | nice. | |
| 23:44:35 | gmann | and all pagination param are single_param.+1 | |
| 23:51:50 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add Selection objects https://review.openstack.org/499239 | |
| 23:51:51 | openstackgerrit | Ed Leafe proposed openstack/nova master: Return Selection objects from the scheduler driver https://review.openstack.org/495854 | |
| 23:51:51 | openstackgerrit | Ed Leafe proposed openstack/nova master: Modify select_destinations() to return objects and alts https://review.openstack.org/510159 | |
| 23:51:52 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change RPC for select_destinations() https://review.openstack.org/516707 | |
| 23:51:52 | openstackgerrit | Ed Leafe proposed openstack/nova master: Move the claim_resources method to scheduler utils https://review.openstack.org/511357 | |
| 23:51:53 | openstackgerrit | Ed Leafe proposed openstack/nova master: Make conductor pass and use host_lists https://review.openstack.org/511358 | |
| 23:51:53 | openstackgerrit | Ed Leafe proposed openstack/nova master: Refactor the code to check for sufficient hosts https://review.openstack.org/520242 | |
| 23:52:12 | edleafe | mriedem: ^^ refactored | |
| 23:53:38 | efried | jaypipes I find it very confusing that _Provider represents a node in a tree of providers, but ProviderTree actuall represents any number of such trees. | |
| 23:54:12 | efried | Rename ProviderTree to ProviderTrees. Or ProviderBush. | |
| 23:56:38 | edleafe | efried: ProviderVine? | |
| 23:57:06 | efried | edleafe ProvideMeVino | |
| 23:57:24 | edleafe | I like the way you think! | |
| 23:57:38 | eandersson | btw does placement take actual resources into account when scheduling vms on overcommited hosts? | |
| 23:59:04 | eandersson | In Mitaka we are hitting this when the compute is out of memory, but the scheduler still thinks there is room available | |
| 23:59:05 | eandersson | > qemu-kvm: cannot set up guest memory 'pc.ram': Cannot allocate memory | |
| 23:59:08 | openstackgerrit | Eric Fried proposed openstack/nova master: Iterator for a provider tree https://review.openstack.org/520243 | |
| 23:59:28 | efried | jaypipes We're gonna need that ^ | |
| 23:59:42 | mriedem | eandersson: are you setting the reserved space config? | |
| 23:59:51 | eandersson | Yea | |
| #openstack-nova - 2017-11-16 | |||
| 00:00:00 | eandersson | This is with overcommit 1.5 on memory, which is fine in most cases for us | |