| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-03 | |||
| 11:22:19 | openstackgerrit | Chris Dent proposed openstack/nova master: Delete a compute node's resource provider when node is deleted https://review.openstack.org/428375 | |
| 11:24:57 | cdent | johnthetubaguy: responded to you about the 404 thing: it's if not 404, log | |
| 11:25:03 | cdent | so everything unusual is logged | |
| 11:25:17 | johnthetubaguy | cdent: balls, I read that as == | |
| 11:25:35 | cdent | on the bugger scale where do balls fit? | |
| 11:25:43 | johnthetubaguy | its hard to know, lol | |
| 11:26:36 | cdent | I made the exact same mistake where the same idiom was used elsewhere (I think on deleting allocations, but not sure). | |
| 11:26:56 | jroll | morning y'all | |
| 11:26:58 | cdent | johnthetubaguy: i put up a new version of that while you were commenting. I think getting all the previous comments | |
| 11:27:09 | johnthetubaguy | cdent: yeah, just taking a peak | |
| 11:27:10 | jroll | "It seems to be the webob pattern to make a non-2xx response False-y" webob whyyyy | |
| 11:27:14 | cdent | morning jroll, you might want to look at https://review.openstack.org/#/c/428375/ too | |
| 11:27:24 | cdent | that's request not webob doing that | |
| 11:27:25 | jroll | yes, I'm already there :) | |
| 11:27:36 | jroll | ok requests whyyyy | |
| 11:27:46 | jroll | :) | |
| 11:27:49 | cdent | and I would guess they do it because, like, man, requests is http for humans, man | |
| 11:28:34 | cdent | it vaguely makes sense, but it does make for some confusion, and is to me biased | |
| 11:28:56 | cdent | I think response object would only be falsey if the web server couldn't be reached | |
| 11:29:02 | jroll | yeah, it's non-obvious | |
| 11:29:09 | jroll | oh well, learned something new today | |
| 11:29:21 | johnthetubaguy | its better than success being falsey I guess | |
| 11:32:21 | andymccr | if anybody is about to give https://review.openstack.org/#/c/426163/ a review, that'd be really appreciated :) | |
| 11:34:38 | johnthetubaguy | cdent: should we worry about the compute nodes we already deleted, but have entries for in the placement API, I am thinking probably no? | |
| 11:38:49 | cdent | johnthetubaguy: I'm not sure how we'd figure that out in that direction. | |
| 11:39:35 | johnthetubaguy | cdent: good question, I guess we can't list the resource providers by host as such, only node? | |
| 11:40:54 | johnthetubaguy | cdent: I do wonder about a nova-manage tool to "re-sync" placement, and check for any odd things, its something that could live in such a tool. Although creating that tool feels a bit too much like giving up. | |
| 11:44:19 | jroll | johnthetubaguy: left a comment, I'm thinking we throw people a release note about that, at least? | |
| 11:44:28 | jroll | patch lgtm otherwise though | |
| 11:46:04 | cdent | johnthetubaguy: yeah the host, node stuff is not something that the placement side knows. it just knows resource providers. it's up to the "nova" side to do that jiggery pokery | |
| 11:47:42 | cdent | jroll: That release note seems potentially very confusing and scary for people and might lead to random deletions against the api? | |
| 11:48:31 | jroll | cdent: yeah, we can add big scary warnings or something | |
| 11:48:56 | jroll | warning: this only applies if you're doing CD by the day/week | |
| 11:48:59 | jroll | idk maybe it's fine | |
| 11:49:06 | jroll | feels like we should tell people somehow though | |
| 11:57:55 | johnthetubaguy | cdent: jroll: oh, so this is affecting ironic already, I kept thinking it wasn't | |
| 11:58:08 | johnthetubaguy | wires crossed in my head | |
| 11:58:11 | jroll | johnthetubaguy: yeah, since a few days ago, I believe | |
| 11:59:54 | johnthetubaguy | surely since last cycle when we started reporting the inventory? | |
| 12:00:14 | johnthetubaguy | or did we skip ironic till really recently? | |
| 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 | |