| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-02 | |||
| 14:30:53 | jroll | I have not independently reproduced it | |
| 14:31:01 | jroll | I'm just trying to answer question while vdrok is at lunch :) | |
| 14:31:27 | vdrok | jaypipes: so, I tried that, return (nodename not in self.compute_nodes) only in the disabled, restarted both computes, still the same | |
| 14:31:57 | jroll | mriedem: nah, I think all the placement/allocations stuff is working fine, except that the node being selected doesn't exist, so spawn() explodes | |
| 14:32:49 | mriedem | what is selecting the node? the scheduler? | |
| 14:33:31 | jroll | I assume so | |
| 14:33:36 | jroll | that's what selects things, right | |
| 14:33:50 | vdrok | mriedem: I added a bit more log from the n-cpu | |
| 14:33:50 | edleafe | jroll: jaypipes: so maybe we need to also add a call to the placement API after https://github.com/openstack/nova/blob/master/nova/objects/compute_node.py#L324 to delete the compute node as a resource provider? | |
| 14:33:52 | mriedem | i guess if everything is fully master then the filter scheduler will use placement | |
| 14:34:40 | jroll | yeah, looks recent enough for placement api, that's why I bring it up :) | |
| 14:35:11 | jroll | edleafe: that seems sensible to my feeble brain | |
| 14:35:14 | mriedem | edleafe: then the compute node object has to use the scheduler client... | |
| 14:35:21 | vdrok | now going to lunch for real :) | |
| 14:35:34 | mriedem | i'd rather we just call the scheduler client to delete the RP after deleting the CN here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6585 | |
| 14:35:44 | mriedem | since the compute manager already has it's grubby hands into that code | |
| 14:36:58 | edleafe | mriedem: yeah, makes sense | |
| 14:40:32 | mriedem | as far as i can tell, we only ever delete inventories or allocations, but never the actual resource provider | |
| 14:40:53 | mriedem | and since the CN is the RP, and we delete the CN, i guess we need to also delete the RP | |
| 14:41:47 | mriedem | i'm going to head into the office, bbiab | |
| 14:41:52 | cdent | yes | |
| 14:42:07 | cdent | (the delete the rp) | |
| 14:56:13 | mdbooth | lyarwood: I've reviewed the swap_volume change. Haven't looked at the tests yet, btw. Doing that now. | |
| 14:59:35 | lyarwood | mdbooth: thanks again, working on it now. | |
| 15:07:27 | mriedem | please all be fixed, please all be fixed... | |
| 15:07:29 | mriedem | no whammies | |
| 15:07:57 | lyarwood | mriedem: my fix is failing in check, looking now | |
| 15:10:59 | lyarwood | mriedem: http://logs.openstack.org/67/427267/2/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/0f048f1/console.html#_2017-02-02_15_03_33_671655 - sorry if I've missed a previous discussion about this still being broken btw | |
| 15:12:37 | mriedem | lyarwood: another fix for that is in the gate | |
| 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 | |