| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-17 | |||
| 19:24:20 | dansmith | mriedem: also note the use of samehost=foo will require SameHostFilter to be enabled, which isn't by default | |
| 19:26:50 | mriedem | yeah | |
| 19:28:47 | openstackgerrit | Chris Friesen proposed openstack/nova master: WIP: Fail fast if changing image on a volume-backed server rebuild https://review.openstack.org/520660 | |
| 19:31:09 | fried_rice | leakypipes Talk to me about how the resources from get_available_resource are dealt with today, vis-à-vis placement. | |
| 19:31:34 | fried_rice | Do we *just* peel out the CPU, mem, and disk, and let the rest percolate through the old-school way? | |
| 19:31:45 | leakypipes | fried_rice: depends on the virt drive. | |
| 19:31:47 | leakypipes | driver... | |
| 19:32:11 | leakypipes | fried_rice: for libvirt, yes, that's what we do. for ironic, we also handle the custom resource class if the Ironic node's resource_class attribute is set. | |
| 19:32:58 | leakypipes | fried_rice: get_available_resource() is still called for both of those drivers, though. the result of get_available_resource() is used to track NUMA and PCI device resources. | |
| 19:34:15 | fried_rice | leakypipes The ironic thing is done via get_inventory, though, nah? | |
| 19:34:41 | leakypipes | fried_rice: yes, sorry, that's what I thought you were asking about. | |
| 19:35:35 | leakypipes | fried_rice: we don't "peel anything out" of the get_available_resource() response. rather, the get_available_resource() return is simply ignored by placement/reportclient. | |
| 19:36:10 | fried_rice | leakypipes Eh? /me scours code... | |
| 19:36:31 | fried_rice | leakypipes I can easily see how one would convert from get_inventory() to update_provider_tree() -- what I'm wondering about (more long term, really) is how/when one would be able to get rid of get_available_resource() altogether. | |
| 19:36:40 | fried_rice | Sounds like this series won't be the thing that allows that to happen. | |
| 19:37:07 | leakypipes | fried_rice: no, it won't. we will need to handle NUMA stuff before that's a reality. | |
| 19:37:33 | leakypipes | fried_rice: but no *new* code is being allowed in the get_available_resource() calls... | |
| 19:39:18 | fried_rice | leakypipes Humph, indeed I can't find where get_available_resource is parlayed into placement inventory -- but then how does the scheduler work for non-get_inventory()-implementin virt drivers? | |
| 19:40:53 | openstackgerrit | Merged openstack/nova master: begin refactor AllocCandidates._get_by_filters() https://review.openstack.org/516778 | |
| 19:41:03 | fried_rice | leakypipes Woot! ^ | |
| 19:41:47 | leakypipes | :) | |
| 19:42:11 | leakypipes | fried_rice: I think all virt drivers implement get_inventory() by now. | |
| 19:42:24 | fried_rice | leakypipes Nope - PowerVM for sure not. | |
| 19:42:30 | fried_rice | in tree or out of tree. | |
| 19:43:18 | fried_rice | leakypipes Yeah, and yet when I look in the placement DB on my PowerVM compute node, I see a RP entry for the compute node and inventory for VCPU, MEMORY_MB, and DISK_GB. | |
| 19:43:22 | fried_rice | At least, I thought I did. Looking again... | |
| 19:44:07 | leakypipes | fried_rice: then there would be no resource providers in the placement/API DB. which AllocationCandidates.get_by_filters() would return [], [], which will trigger shceudler to do a ComputeNodeList.get_all() which will use the vcpus, memory_mb, and local_gb fields in the cell compute_nodes tables and those fields are populated with the get_available_resource() results. | |
| 19:44:54 | fried_rice | leakypipes And that... populates the placement DB with those values? | |
| 19:45:05 | fried_rice | Cause I confirmed | |
| 19:45:36 | fried_rice | leakypipes http://paste.openstack.org/show/626656/ | |
| 19:46:34 | fried_rice | leakypipes And no get_inventory in https://github.com/openstack/nova-powervm/blob/master/nova_powervm/virt/powervm/driver.py | |
| 19:47:23 | leakypipes | fried_rice: https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L824 | |
| 19:47:52 | leakypipes | fried_rice: there is a hack in the scheduler client that sets the inventory based on the vcpus, memory_mb and local_gb of the compute node if the get_inventory() method is missing. | |
| 19:48:10 | leakypipes | fried_rice: see the code comment directly above that... | |
| 19:48:11 | leakypipes | # this code branch | |
| 19:48:11 | leakypipes | # format that the placement API expects and we'll be able to remove | |
| 19:48:11 | leakypipes | # Eventually all virt drivers will return an inventory dict in the | |
| 19:48:37 | fried_rice | leakypipes _compute_node_to_inventory_dict - booya. | |
| 19:48:48 | fried_rice | leakypipes Thanks for keeping me sane. | |
| 19:49:00 | leakypipes | no problemo. | |
| 19:50:26 | dansmith | mriedem: um.. I can't update a service to disable it in 2.38 | |
| 19:50:44 | dansmith | mriedem: and the schema makes it sound like we can't except in >=2.53, but... we've been able to do that forever, no? | |
| 19:51:35 | dansmith | v211 has a disabled_reason=, but no status= property for me to set to disabled... | |
| 19:57:05 | dansmith | ah, I guess it was an action before | |
| 20:02:27 | openstackgerrit | Merged openstack/nova master: build ProviderSummary objects in sep function https://review.openstack.org/516779 | |
| 20:22:31 | mriedem | dansmith: yeah, 2.53 is a PUT api | |
| 20:22:41 | dansmith | yeah, I didn't realize | |
| 20:22:47 | dansmith | I'm close to a prototype to push up, fyi | |
| 20:24:45 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add Selection objects https://review.openstack.org/499239 | |
| 20:24:46 | openstackgerrit | Ed Leafe proposed openstack/nova master: Return Selection objects from the scheduler driver https://review.openstack.org/495854 | |
| 20:24:46 | openstackgerrit | Ed Leafe proposed openstack/nova master: Refactor the code to check for sufficient hosts https://review.openstack.org/520242 | |
| 20:24:47 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change RPC for select_destinations() https://review.openstack.org/516707 | |
| 20:24:47 | openstackgerrit | Ed Leafe proposed openstack/nova master: Modify select_destinations() to return objects and alts https://review.openstack.org/510159 | |
| 20:24:48 | openstackgerrit | Ed Leafe proposed openstack/nova master: Move the claim_resources method to scheduler utils https://review.openstack.org/511357 | |
| 20:24:48 | openstackgerrit | Ed Leafe proposed openstack/nova master: Make conductor pass and use host_lists https://review.openstack.org/511358 | |
| 20:26:00 | mriedem | dansmith: ok. i had to step away for a bit to clear my head - hadn't taken a break since i started today. i think i'm going to write a regression test for https://bugs.launchpad.net/nova/+bug/1732947 since i think rebuild + bfv with new image is something we need to fix anyway | |
| 20:26:00 | openstack | Launchpad bug 1732947 in OpenStack Compute (nova) "volume-backed instance rebuild with no image change is still going through scheduler" [Undecided,In progress] - Assigned to Chris Friesen (cbf123) | |
| 20:26:13 | mriedem | and rebuild + bfv with original image for that matter | |
| 20:26:27 | dansmith | okay | |
| 20:27:54 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Alternate fix for validating image on rebuild https://review.openstack.org/521186 | |
| 20:28:29 | dansmith | mriedem: when you get a sec, let me explain my thinking in that test to make sure it's sound | |
| 20:30:47 | mriedem | i think i get it | |
| 20:31:04 | mriedem | we still have the allocation doubling up issue, but that's a separate problem | |
| 20:32:44 | dansmith | yeah | |
| 20:33:09 | dansmith | so if this is what you want, I need to go change the rest of the filters that are placement related to skip like this one does | |
| 20:36:57 | dansmith | mriedem: there's also the question of just doing this, or putting a straight revert in front | |
| 20:37:07 | dansmith | as this is, I'd have to backport the broken one first before I can backport this | |
| 20:37:24 | dansmith | which I can do for sure, but I'd vote to cleanly revert and re-fix | |
| 20:38:38 | openstackgerrit | Eric Fried proposed openstack/nova master: Scheduler set_inventory_for_provider does nested https://review.openstack.org/520643 | |
| 20:38:38 | openstackgerrit | Eric Fried proposed openstack/nova master: ProviderTree.uuid_set() https://review.openstack.org/520243 | |
| 20:38:39 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_tree https://review.openstack.org/520663 | |
| 20:38:39 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_aggregates https://review.openstack.org/521097 | |
| 20:38:40 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098 | |
| 20:38:40 | openstackgerrit | Eric Fried proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756 | |
| 20:38:41 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187 | |
| 20:38:41 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246 | |
| 20:38:44 | fried_rice | leakypipes Nits all fixed up ^ | |
| 20:39:07 | leakypipes | fried_rice: ah, you went with uuid_set(). noice. | |
| 20:39:13 | fried_rice | leakypipes I took the hint :) | |
| 20:39:23 | leakypipes | fried_rice: it was a gentle hint. :) | |
| 20:39:38 | mriedem | dansmith: comments inline | |
| 20:40:00 | mriedem | dansmith: and yeah, not sure about the revert at this point | |
| 20:40:53 | fried_rice | leakypipes This -1 was on the wrong patch btw - it stuck around after the rebase: https://review.openstack.org/#/c/520663/ | |
| 20:41:12 | fried_rice | leakypipes ...just so you didn't bypass that guy for thinking you'd already seen it. | |
| 20:41:47 | leakypipes | fried_rice: gotcha. | |
| 20:41:59 | leakypipes | fried_rice: soon as tests complete, will re-review the series. | |
| 20:42:06 | fried_rice | leakypipes Dig | |
| 20:42:29 | openstack | Launchpad bug 1732947 in OpenStack Compute (nova) "volume-backed instance rebuild with no image change is still going through scheduler" [Undecided,In progress] - Assigned to Chris Friesen (cbf123) | |
| 20:42:29 | mriedem | i'll work on the recreate test for https://bugs.launchpad.net/nova/+bug/1732947 and come back | |
| 20:45:43 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add regression test for rebuild with new image doubling allocations https://review.openstack.org/521153 | |
| 20:45:54 | dansmith | mriedem: abstractmethod implies that we change the base filter to inherit from ABCMeta which is more change | |
| 20:46:18 | dansmith | mriedem: we could also flip this and make the filters all skip if we're doing this policy check, unless they're opt-in as policy filters | |
| 20:46:40 | dansmith | mriedem: and then just have them set POLICY=True in their class definition | |
| 20:46:58 | mriedem | yeah i was thinking we could avoid the abc if we just have a default in the base and override in the subclasses | |
| 20:47:03 | mriedem | like we do for calling placement for the scheduler drivers | |
| 20:47:25 | dansmith | mriedem: and you want the default to be not-policy so we just override the ones that are policy? | |
| 20:47:55 | mriedem | if that means fewer overrides, then i think yes | |
| 20:48:04 | dansmith | I think it will | |
| 20:48:05 | mriedem | we want the default to be whatever the majority of filters want | |
| 20:48:06 | mriedem | ok | |