| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-02 | |||
| 15:13:03 | lyarwood | mriedem: ah great, sorry I missed that fly past | |
| 15:13:35 | mriedem | lyarwood: this one https://review.openstack.org/#/c/427775/ | |
| 15:13:49 | mriedem | 2 in the gate | |
| 15:17:34 | mriedem | do we have a definition of barber-pole sorting anywhere? | |
| 15:18:49 | mriedem | just means it wraps around from cell to cell right? | |
| 15:19:05 | mriedem | "striped across the cell boundaries" | |
| 15:19:40 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix typo in cells v2 ocata reno https://review.openstack.org/428198 | |
| 15:22:04 | jaypipes | edleafe: when (if ever) do compute_nodes table records get deleted? | |
| 15:24:12 | jaypipes | mriedem: more generally, I wonder when we ever delete compute nodes... | |
| 15:24:48 | mriedem | in the case of ironic, i guess when they stop reporting available nodes | |
| 15:25:21 | mriedem | jaypipes: also when the nova-compute service is deleted via the api i think | |
| 15:25:25 | mriedem | will have to look | |
| 15:25:31 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Improve assertJsonEqual error reporting https://review.openstack.org/428199 | |
| 15:25:40 | mriedem | i remember fixinga 404 in the api around that recently | |
| 15:25:53 | jaypipes | mriedem: that would be a big mistake in the case of Ironic then... (since service != compute_node for Ironic) | |
| 15:26:15 | mriedem | https://review.openstack.org/#/c/406627/ was the 404 i was thinking of | |
| 15:26:25 | mriedem | "Compute node resources must currently be deleted manually in the database" | |
| 15:27:45 | edleafe | jaypipes: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6585 | |
| 15:27:47 | mriedem | yeah | |
| 15:27:47 | mriedem | but we do have this https://github.com/openstack/nova/blob/13.1.2/nova/compute/manager.py#L6513 | |
| 15:27:51 | edleafe | jaypipes: which in turn calls https://github.com/openstack/nova/blob/master/nova/objects/compute_node.py#L324 | |
| 15:27:52 | jaypipes | mriedem: yes, and I had some strong words on that patch :) | |
| 15:27:59 | diana_clarke | sfinucan: I heard you like docs ;) https://review.openstack.org/#/c/411441/ | |
| 15:28:11 | diana_clarke | sfinucan: (but only if you're not otherwise busy) | |
| 15:28:16 | mriedem | http://www.thereluctanttecchie.com/openstack-removing-a-compute-node-in-icehouse/ | |
| 15:28:16 | jaypipes | edleafe: doink. | |
| 15:28:26 | jaypipes | edleafe: right on, brother, nice detective work. | |
| 15:28:39 | jaypipes | edleafe: we need to have the resource tracker do that, not the compute manager. | |
| 15:28:45 | jaypipes | edleafe: I can work on a patch. | |
| 15:29:02 | edleafe | jaypipes: already working on it | |
| 15:29:03 | jaypipes | edleafe: nice find. | |
| 15:29:06 | mriedem | " The fact that the crap hypervisor-list/hypervisor-show REST APIs are still manually attempting to conflate a ComputeNode with a Service is a problem that should be fixed entirely in the REST API, since it is the REST API that is borked." | |
| 15:29:10 | jaypipes | edleafe: oh? even better :) | |
| 15:29:11 | mriedem | those strong words? :) | |
| 15:29:22 | jaypipes | mriedem: heh, yes sir. | |
| 15:29:37 | edleafe | jaypipes: but it's the manager that has to make the call. I don't see the benefit in adding the RT as an indirection layer | |
| 15:29:55 | jaypipes | edleafe: because the RT has the placement client, not the manager. | |
| 15:30:09 | jaypipes | edleafe: and the RT has all the compute node objects now, not the manager. | |
| 15:30:11 | mriedem | the compute manager is the one looking for orphaned computes | |
| 15:30:14 | mriedem | not the RT | |
| 15:30:16 | edleafe | jaypipes: the manager has the scheduler client | |
| 15:30:38 | mriedem | unless you just move that into RT.update_available_resource | |
| 15:30:49 | jaypipes | mriedem: all of that update_available_resource() code in the manager should be in the RT, IMHO. | |
| 15:31:07 | sfinucan | diana_clarke: ack | |
| 15:31:09 | edleafe | jaypipes: we can do scheduler_client.report_client like we added to RT | |
| 15:32:10 | jaypipes | mriedem: jinx. | |
| 15:32:11 | edleafe | mriedem: that sounds clunky to me | |
| 15:32:11 | mriedem | we could build a new method into the RT to remove it if that's cleaner | |
| 15:32:11 | mriedem | so orphaned and we delete the CN record | |
| 15:32:11 | mriedem | and in this case, it's a not available node | |
| 15:32:11 | mriedem | for an available node | |
| 15:32:11 | mriedem | because we only call rt.update_available_resource(context, nodename) | |
| 15:32:11 | mriedem | it's a chicken/egg, | |
| 15:32:11 | mriedem | jaypipes: well, | |
| 15:32:12 | edleafe | there's nothing magic about the client | |
| 15:32:21 | mriedem | either way, i'm totally fine with a quick rt.report_client.delete_resource_provider from the compute manager in ocata if it gets the job done and saves time | |
| 15:32:33 | jaypipes | edleafe, mriedem: what I'm saying is that all the code in the compute manager's update_available_resource() method should be moved to the RT now. There's no reason to have that in the compute manager. | |
| 15:32:34 | mriedem | and a TODO to move that into RT later | |
| 15:32:46 | mriedem | jaypipes: *now* is not the day to do it | |
| 15:32:49 | jaypipes | mriedem: sure, I'm also fine with a quick fix. | |
| 15:32:52 | jaypipes | mriedem: understood. | |
| 15:32:59 | edleafe | mriedem: gimme a few to get something done | |
| 15:33:07 | jaypipes | edleafe: cheers | |
| 15:33:08 | edleafe | all this IRC chat is slowing me down :) | |
| 15:33:43 | jaypipes | mriedem: in that future cleanup, I will also change the InstanceList.get_all_by_host_and_nodename() to be an InstanceList.get_all_by_host() and fix the performance issues with the Ironic RT once and for all. | |
| 15:33:59 | mriedem | yeah i guess compute manager has self.scheduler_client = scheduler_client.SchedulerClient() | |
| 15:34:04 | jaypipes | mriedem: that was the end goal of moving all the compute_node -> compute_nodes handling in the RT. | |
| 15:34:25 | mriedem | jaypipes: ok | |
| 15:34:53 | jaypipes | mriedem: in the meantime, lemme look at the latest patch from edleafe on the Ironic inventory management. | |
| 15:35:40 | mriedem | the harder part of this change is probably going to be the method in the scheduler to remove the RP b/c we'll also have to make sure it doesn't have allocations and inventory in order to delete the RP, else it will 409 | |
| 15:36:34 | jaypipes | mriedem: yup | |
| 15:36:44 | mriedem | diana_clarke: wow nice catch L( | |
| 15:36:45 | mriedem | :) | |
| 15:36:52 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix typo in cells v2 ocata reno https://review.openstack.org/428198 | |
| 15:36:53 | edleafe | mriedem: add 'force=True'? | |
| 15:37:02 | mriedem | edleafe: is that a thing in the REST API? | |
| 15:37:17 | jaypipes | edleafe: I assigned that bug to you, FYI | |
| 15:37:26 | jaypipes | mriedem: no. | |
| 15:37:26 | edleafe | mriedem: nope - I was joking | |
| 15:37:34 | edleafe | forgot the smiley | |
| 15:37:48 | jaypipes | edleafe: nah, it's easy enough to write code that looks at existence of allocations and deletes them, then inventory, then resource provider. | |
| 15:37:58 | jaypipes | heh | |
| 15:38:06 | mriedem | we already have methods to delete the allocations and inventory, so probably can just re-use those | |
| 15:38:19 | edleafe | jaypipes: it's the same code; doing it in the client just makes more HTTP calls | |
| 15:38:28 | edleafe | mriedem: yep, that's the plan | |
| 15:38:38 | jaypipes | mriedem: indeed. in fact, https://review.openstack.org/#/c/404472 has examples of using those exact calls :) | |
| 15:39:18 | mriedem | i'm going to need to see johnthetubaguy be happy with that before i can look at it probably | |
| 15:39:33 | jaypipes | edleafe: understood. I'd prefer to keep a single HTTP call to a single thing unless there's a strong argument not to. | |
| 15:39:37 | mriedem | jaypipes: we also have 2 bugs for disk inventory/allocation in the report client | |
| 15:39:49 | jaypipes | s/to a single thing/to do a single thing/ | |
| 15:39:55 | jaypipes | mriedem: links pls? | |
| 15:40:02 | mriedem | https://review.openstack.org/#/c/428120/ | |
| 15:40:10 | edleafe | jaypipes: there is no HTTP method that will remove a RP and all of its associated invetory and allocations | |
| 15:40:10 | openstack | Launchpad bug 1659266 in OpenStack Compute (nova) "Disk allocation for instance is not good with swap" [High,Confirmed] - Assigned to Tanvir Talukder (tanvirt16) | |
| 15:40:10 | mriedem | https://bugs.launchpad.net/nova/+bug/1659266 | |
| 15:40:28 | edleafe | jaypipes: so adding one will mean a new microversion :( | |
| 15:40:29 | jaypipes | edleafe: I know, and I think that's appropriate. | |
| 15:41:05 | edleafe | I would certainly prefer to add it that way, but with RC1... | |
| 15:41:16 | jaypipes | edleafe: meaning DELETE /resource-providers/<UUID> should return a 409 if there are existing allocations and inventories. The user should be required to DELETE /allocations|inventories in order to perform the DELETE /r-p | |
| 15:41:35 | edleafe | jaypipes: of course | |