| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-02 | |||
| 13:46:23 | mriedem | i see that debug log message hit 3 times in the n-cond logs on a run of that patch http://logs.openstack.org/75/427775/1/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/c906c15/logs/screen-n-cond.txt.gz#_2017-02-01_16_14_03_273 | |
| 13:46:31 | mriedem | so it's obviously catching the issue | |
| 13:46:51 | mriedem | dansmith: bauzas: ^ so i'm thinking we should probably just get that in given the time | |
| 13:53:10 | cdent | mriedem: unless you're already on it, I'll fix the nits in https://review.openstack.org/#/c/427367 | |
| 13:53:48 | mriedem | i'll get it quick | |
| 13:55:41 | openstackgerrit | Matt Riedemann proposed openstack/nova master: doc: add upgrade notes to the placement devref https://review.openstack.org/427367 | |
| 13:55:42 | mriedem | sfinucan: bauzas: ^ fixed the typo | |
| 13:56:20 | bauzas | mriedem: okay, let's merge https://review.openstack.org/#/c/427775/1 then | |
| 13:57:23 | sfinucan | mriedem: Job. Done. | |
| 13:57:26 | mriedem | i wonder if it would be better to destroy the build request right before creating the instance? | |
| 13:57:53 | mriedem | but, i don't really want to ponder that today | |
| 13:58:17 | bauzas | mriedem: possibly | |
| 13:58:24 | bauzas | mriedem: the main issue I see here is timing | |
| 13:58:45 | bauzas | so let's merge your change is quite impactless and think about the best approach for Pike | |
| 14:00:41 | johnthetubaguy | mriedem: FWIW, I think its best to create the instance first, so we don't "loose" the instance, although marking the build request in someway that stops deletes might make sense, maybe thats the cell mapping entry? | |
| 14:01:24 | johnthetubaguy | hmm, I can't 100% remember the ordering now | |
| 14:02:07 | johnthetubaguy | mriedem: this one the openstack ansible folks I spotted feels nasty, but we totally could back port it later, I believe | |
| 14:02:47 | mriedem | johnthetubaguy: https://review.openstack.org/#/c/428120/ right? | |
| 14:02:50 | mriedem | i left some comments in there | |
| 14:03:00 | mriedem | there is a bug report i think that fits the same thing already | |
| 14:03:03 | mriedem | i've marked it rc1 | |
| 14:06:36 | alex_xu | johnthetubaguy: I think gmann is right on https://review.openstack.org/#/c/428071, we agreed on we have filter which match the name in the API representation, but that will be done in the future with microversion. | |
| 14:11:54 | johnthetubaguy | mriedem: the bug is different I think, the bug talks about the allocation, this my bug is about the inventory (due to the reservation) | |
| 14:12:09 | openstack | Launchpad bug 1661258 in OpenStack Compute (nova) "Deleted ironic node has an inventory in nova_api database" [Undecided,New] | |
| 14:12:09 | jroll | mriedem: fyi this one looks fun https://bugs.launchpad.net/nova/+bug/1661258 | |
| 14:13:41 | mriedem | jroll: damn | |
| 14:13:52 | jroll | it might actually be easy | |
| 14:13:53 | mriedem | jroll: will need cdent, edleafe and/or jaypipes to look at that one | |
| 14:14:02 | jroll | mriedem: thank you for pinging them :P | |
| 14:14:17 | mriedem | i was thinking the periodic update would remove the inventory for the removed node but vdrok said in the bug that it didn't | |
| 14:14:22 | cdent | joy | |
| 14:15:34 | edleafe | mriedem: I can take a look at that | |
| 14:15:57 | jroll | right, I assume we're missing something like this for inventory https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6577 | |
| 14:18:50 | mriedem | jroll: yeah that looks reasonable | |
| 14:19:02 | mriedem | jaypipes refactored a ton of that code so he'll know best | |
| 14:19:15 | jaypipes | jroll, vdrok, mriedem, edleafe, cdent: I have a suspicion on cause of that bug. | |
| 14:19:31 | edleafe | jaypipes: do tell | |
| 14:20:11 | jaypipes | jroll, vdrok, mriedem, edleafe, cdent: Ironic virt driver is probably returning False for node_is_available() and therefore when update_available_resource() runs, we never even get to the call for InstanceList.get_by_host_and_node() and therefore never clean up the now-deleted instance. | |
| 14:20:48 | mriedem | sdague: how do you feel about this? https://review.openstack.org/#/c/426926/ | |
| 14:20:49 | jaypipes | mriedem: remember when you asked me to change RT.disabled(nodename) to account for the Ironic-only node_is_available() call? :) | |
| 14:20:57 | mriedem | sdague: because i'd like to get the dependent nova change in for ocata | |
| 14:21:15 | mriedem | jaypipes: vaguely | |
| 14:21:35 | mriedem | jaypipes: i'm pretty sure i was drunk reviewing most of that refactor series :) | |
| 14:21:45 | jroll | jaypipes: this is about deleted nodes, not deleted instances, to be clear | |
| 14:22:11 | jroll | jaypipes: so it wouldn't even be returned in get_available_nodes() | |
| 14:22:18 | jaypipes | vdrok: I asked for you to do a flash-patch in the bug above. | |
| 14:22:34 | cdent | is running two nova-computes on the same machine normal and/or an important consideration in this bug? | |
| 14:23:01 | jaypipes | jroll: right, but the inventory won't get cleaned up in the placement DB if the node_is_available() returns False here: | |
| 14:23:02 | jaypipes | https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L414 | |
| 14:23:02 | vdrok | jaypipes: ok, thanks, will do after lunch | |
| 14:23:12 | jaypipes | cdent: no. | |
| 14:23:26 | sdague | mriedem: yeh, that seems fine. | |
| 14:23:41 | sdague | mriedem: under what circumstances does that error out? | |
| 14:24:09 | jroll | jaypipes: yeah, it'll definitely return False there if the node is deleted | |
| 14:24:40 | mriedem | sdague: would have to dig into nova-status code for specifics, but things like not being able to talk to the placement api | |
| 14:24:49 | sdague | mriedem: yeh, seems fine | |
| 14:25:42 | mriedem | no cell mappings, no cell0, no host mappings when there are computes, etc | |
| 14:25:43 | mriedem | thanks | |
| 14:26:06 | edleafe | jroll: So if the node is deleted, the problem is just orphaned invntory, right? IOW, it isn't in the DB anymore, so it won't get returned from a placement GET | |
| 14:26:38 | jroll | edleafe: AIUI it *is* being returned from the placement API | |
| 14:27:10 | mriedem | the error in the bug report was missing some details, like what the actual build failure was | |
| 14:27:33 | jroll | mriedem: fault | {"message": "Node 6cc8803d-4e77-4948-b653-663d8d5e52b7 could not be found. (HTTP 404)", "code": 500, "details": " File \"/opt/stack/nova/....... | |
| 14:27:38 | jroll | right at the top | |
| 14:27:57 | mriedem | yeah, but | |
| 14:28:08 | mriedem | that doesn't really have anything to do with placement... | |
| 14:28:22 | mriedem | it's in _build_and_run_instance | |
| 14:28:41 | jroll | well | |
| 14:28:47 | jroll | it selected a node that doesn't exist in ironic | |
| 14:29:23 | jroll | it also doesn't exist in the compute_nodes table, but does exist in inventory | |
| 14:29:37 | mriedem | unless it's blowing up here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1905 | |
| 14:30:26 | edleafe | jroll: so it's gone from the compute_nodes table in the cell, but not in the resource_providers table in the API/placement db | |
| 14:30:39 | edleafe | jroll: So placement still thinks it's there | |
| 14:30:40 | jroll | edleafe: AIUI, yes | |
| 14:30:44 | jroll | keep in mind I did not file this bug | |
| 14:30:50 | edleafe | jroll: ack | |
| 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... | |