| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-03 | |||
| 12:00:40 | jroll | johnthetubaguy: oh. ohhhhhh. | |
| 12:00:46 | jroll | damn | |
| 12:00:57 | jroll | yeah, we only noticed it when scheduling switched to inventory | |
| 12:01:12 | johnthetubaguy | oh, right, its only impacting since we started caring about whats in there | |
| 12:01:13 | jroll | but I guess the real problem might be much longer eh | |
| 12:01:33 | jroll | I guess we could spin up a newton install and test this easily | |
| 12:02:51 | cdent | johnthetubaguy: are you taking typing lessons from me "I was forgetting this could already an issue in production for folks, we should do that thing." | |
| 12:03:15 | johnthetubaguy | heh | |
| 12:03:53 | cdent | So how should this reno express itself? Do we explain just the issue or the issue and the hack for how to deal with it? Or something else? | |
| 12:04:45 | johnthetubaguy | cdent: its probably worth saying the problem you will see with iroinc and placement if you don't clear out old deleted nodes | |
| 12:05:15 | johnthetubaguy | cdent: its tempting to hack up some tool to help fix that, but we good to stop the bleeding with this patch anyways | |
| 12:06:23 | cdent | Just so I'm clear on the situation how would that hack operate? How do you know which things on the placement api side to delete? | |
| 12:06:50 | johnthetubaguy | because they are not in the current compute node list? I am not 100% sure actually | |
| 12:07:16 | jroll | doesn't resource_provider.uuid == ironic_node.uuid here? | |
| 12:09:28 | johnthetubaguy | cdent: so compute node is soft_delete, so if they don't trim that table, we could double check from soft deleted compute nodes to double check they are all removed | |
| 12:12:11 | cdent | but if they do, then they truly are orphans, unless the rp.name is useful in some fashion? | |
| 12:14:13 | cdent | so the hack is for each get_all_soft_delete_compute_nodes; try_to_delete_rp_with_same_uuid() | |
| 12:14:32 | johnthetubaguy | name is hypervisor_hostname I think | |
| 12:14:49 | johnthetubaguy | https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L630 | |
| 12:16:09 | jroll | and hypervisor_hostname is the ironic node uuid | |
| 12:16:30 | johnthetubaguy | can you get a list of the deleted uuids though? | |
| 12:16:46 | johnthetubaguy | I mean you can get all the current ones and spot the difference | |
| 12:16:50 | johnthetubaguy | maybe thats the best way | |
| 12:16:57 | jroll | yeah that'd be the only way | |
| 12:17:19 | cdent | blechy | |
| 12:17:32 | jroll | for cn in compute_nodes: if ironic.get_node(cn.hypervisor_hostname) gives a 404: deleted_nodes.append(cn.hypervisor_hostname) | |
| 12:17:52 | jroll | delete from resource_providers where name in $deleted_nodes; | |
| 12:17:52 | johnthetubaguy | but we already deleted the compute node | |
| 12:18:08 | jroll | oh right, s/compute_nodes/inventory thing | |
| 12:18:14 | jroll | you get the idea :) | |
| 12:18:16 | johnthetubaguy | ah, gotcha | |
| 12:18:28 | cdent | "inventory thing"? | |
| 12:18:42 | cdent | (hasn't had sufficient coffee to get the idea) | |
| 12:18:44 | jroll | heh | |
| 12:19:11 | jroll | for rp in resource_providers: if ironic.get_node(rp.name).status_code == 404: deleted_nodes.append(rp.name) | |
| 12:19:14 | jroll | cdent: ^ | |
| 12:19:19 | johnthetubaguy | yeah, that | |
| 12:20:31 | cdent | isn't that proposing deleting all resource providers that are not ironic nodes? | |
| 12:20:34 | johnthetubaguy | feels like we should add a nova-manage to do that | |
| 12:20:57 | jroll | cdent: yes, more sanity checks are needed for deployments that include not-ironic and such | |
| 12:20:59 | johnthetubaguy | cdent: yeha, I guess we have to mix in compute nodes too | |
| 12:21:11 | jroll | if I was going for 100% ready to ship I wouldn't be writing this in irc :) | |
| 12:21:23 | johnthetubaguy | listing all compute nodes, would do it, rather than iroinc I guess | |
| 12:21:26 | cdent | and any other resource providers someone has chosen to create outside the context of the resource tracker | |
| 12:21:50 | cdent | we cannot assume the list of resource providers is only things "we" have created | |
| 12:22:03 | jroll | true | |
| 12:22:27 | johnthetubaguy | well, feels liek we should add some user concepts in, when we can't assume that | |
| 12:23:08 | jroll | maybe the reno is just "if you get build failures trying to schedule to a non-existent node, check *waves hands* stuff, you might need to delete the RP" | |
| 12:23:13 | cdent | I gotta break away for several minutes, perhaps we can all percolate on this for a small window of time and we'll know something after that? | |
| 12:23:24 | cdent | jroll++ :) | |
| 12:24:58 | johnthetubaguy | yeah, a vague reno is better than no reno or fix getting in | |
| 12:25:26 | cdent | bbs | |
| 12:28:21 | BobBall | johnthetubaguy / mriedem: Citrix XenServer CI is back healthy now - can username citrix_xenserver_ci be re-added to the voting group https://review.openstack.org/#/admin/groups/511,members? | |
| 13:06:42 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove redundant bdm serial mangling and saving during swap_volume https://review.openstack.org/427364 | |
| 13:18:02 | johnthetubaguy | BobBall: OK | |
| 13:18:42 | openstackgerrit | Silvan Kaiser proposed openstack/nova master: Removes getfattr from Quobyte Nova driver https://review.openstack.org/428646 | |
| 13:18:59 | johnthetubaguy | BobBall: is a vote on this one coming really soon? https://review.openstack.org/#/c/424428/ | |
| 13:19:34 | BobBall | johnthetubaguy: If by soon you mean 25 minutes ago, then yes | |
| 13:19:53 | BobBall | We commented but didn't having voting rights so no vote :) | |
| 13:25:31 | tdurakov | BobBall: in case of re-roll please add change-log comment to xen migrate data object | |
| 13:26:06 | johnthetubaguy | BobBall: heh, thats soon, cool. turns out you have to click refresh | |
| 13:26:31 | BobBall | tdurakov: Sure will do - what do you mean by re-roll though? | |
| 13:26:39 | tdurakov | BobBall: also, what if I'm migration instances, from 6.5 to 7.0 or vice versa is this field required for both of them? | |
| 13:27:01 | tdurakov | BobBall: resubmit patch | |
| 13:27:08 | tdurakov | in case of ^ | |
| 13:28:05 | johnthetubaguy | tdurakov: I believe this is Nova version related, not XenServer version related | |
| 13:28:07 | BobBall | tdurakov: Migrating from 7.0 to 6.5 isn't supported by XenServer - XAPI will prevent that explicitly. 6.5 to 7.0 will work as the source doesn't look at or use the vif_map, it's prepared by and used by the target. | |
| 13:28:42 | tdurakov | johnthetubaguy: from the but definiton I understood that xen version affect this as well | |
| 13:28:51 | tdurakov | s/but/bug | |
| 13:28:57 | johnthetubaguy | yeah, I see that detail now | |
| 13:28:59 | johnthetubaguy | oops | |
| 13:29:00 | mriedem | cdent: johnthetubaguy: jroll: vdrok: edleafe: on https://review.openstack.org/#/c/428375/ i think we actually should raise a ResourceProviderInUse or some kind of exception if we delete the compute node but can't delete the resource provider, | |
| 13:29:05 | BobBall | johnthetubaguy: XS 7.0 introduced the requirement of the vif map | |
| 13:29:06 | mriedem | because that means we won't reschedule | |
| 13:29:44 | cdent | mriedem: but what would catch it? it's already getting logged as a warning? | |
| 13:30:00 | mriedem | way up in the compute manager | |
| 13:30:03 | johnthetubaguy | mriedem: I thought this was just getting called from some periodic task? | |
| 13:30:07 | mriedem | the thing that caught the NodeNotFound in the bug report | |
| 13:30:09 | mriedem | no | |
| 13:30:12 | mriedem | well, | |
| 13:30:18 | johnthetubaguy | both I guess | |
| 13:30:19 | mriedem | yeah it is | |
| 13:30:29 | mriedem | the periodic task doesn't delete the RP, | |
| 13:30:32 | mriedem | and then the scheduler uses it, | |
| 13:30:44 | mriedem | which then blows up with NodeNotFound in build_and_run_instances | |
| 13:30:48 | mriedem | which triggers a reschedule to another node | |
| 13:30:49 | mriedem | so, | |
| 13:30:56 | mriedem | i guess raising the exception up doesn't help | |
| 13:30:58 | mriedem | fooy | |
| 13:31:04 | cdent | indeed | |
| 13:31:30 | cdent | this is messy. mriedem did you see all the chatter above about how to word a reno note (I linked on the review if not) | |
| 13:31:44 | mriedem | i didn't read that yet | |
| 13:32:17 | mriedem | "btw this optional thing which you might be using might be borking your ironic scheduling but then again if you're not using it so nevermind but just in case you were and hit this thing....beware" | |
| 13:32:23 | mriedem | something like that? ^ | |
| 13:33:15 | cdent | well yeah, exactly, that's pretty much the problem | |
| 13:33:49 | cdent | I'm drafting something now | |
| 13:34:20 | johnthetubaguy | could we use that NodeNotFound error to trigger a check of that resource provider, thinking about it | |
| 13:34:36 | openstackgerrit | Sergey Nikitin proposed openstack/nova-specs master: PCI NUMA Policies https://review.openstack.org/361140 | |
| 13:34:40 | vdrok | if the compute node was deleted but resource provider was not, the bit of deleting resource provider will not be retried by the periodic task? | |
| 13:36:38 | jroll | vdrok: yeah, that could be a problem, maybe we do it the other way (RP first, CN second), and if RP blows up don't delete the compute node either | |