| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-21 | |||
| 22:54:43 | mriedem | which isn't in quenes | |
| 22:54:45 | mriedem | *queens | |
| 22:55:18 | mriedem | there is a pretty beefy ML thread about all of this orphaned provider stuff months back | |
| 22:55:38 | mriedem | i've been slowly polishing these turds | |
| 22:55:54 | melwitt | yeah, sean-k-mooney mentioned that | |
| 22:55:59 | melwitt | the turd polishing | |
| 22:56:07 | mriedem | http://lists.openstack.org/pipermail/openstack-discuss/2019-June/007135.html is the tl;dr of the first mega beef thread | |
| 22:56:25 | mriedem | http://lists.openstack.org/pipermail/openstack-discuss/2019-June/thread.html#7097 | |
| 22:56:37 | melwitt | thank you. my brain is like about to explode so tl;dr is majorly appreciated | |
| 22:56:57 | mriedem | http://lists.openstack.org/pipermail/openstack-discuss/2019-November/thread.html#10642 is the post-ptg summary | |
| 22:57:02 | melwitt | I'm adding a comment to your review just so.... it's there | |
| 22:57:25 | mriedem | yeah so related to this, | |
| 22:57:33 | mriedem | https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:stable/train+topic:bug/1852610 | |
| 22:58:10 | mriedem | and https://review.opendev.org/#/q/status:open+project:openstack/osc-placement+branch:master+topic:story/2006779 | |
| 22:59:10 | melwitt | so you're thinking a change of the ordering and backport to queens is not gonna be viable? I guess you're saying that can't happen in rocky. dansmith said the same thing and I had no understanding of how, but I trust it. the messed up thing is they're also seeing a ResourceProviderCreationFailed on an overcloud (not ironic!) BUT now that I'm thinking more, that must be the service deletion case yeah? | |
| 22:59:31 | melwitt | that you're solving in that patch | |
| 22:59:49 | mriedem | we've backported several pieces of this, and i have those train backports up for part of it as well | |
| 23:00:19 | mriedem | we didn't backport the ironic node uuid = compute node uuid = provider uuid thing to queens because the initial patch had caused some issues and other fallout that i recently fixed as well | |
| 23:00:35 | mriedem | so backporting that ironic / compute node uuid sync stuff to queens would involve a few patches | |
| 23:00:44 | melwitt | we've got customers hitting the ironic case in queens and a non-ironic case in queens. and the former is the orphan cleanup and the latter must be a service deletion issue, *maybe* | |
| 23:01:01 | mriedem | otherwise service delete orphan stuff is bugs and i've been writing these as backportable changes | |
| 23:01:12 | mriedem | bugs since....pike | |
| 23:01:15 | mriedem | i think | |
| 23:01:25 | mriedem | well ocata really | |
| 23:01:40 | melwitt | ok, so you're saying the strategy should be to backport the uuid matcher patch rather than a split out "change the order" patch | |
| 23:02:03 | melwitt | but would that not require some kinda migration actions for already existing compute node records? | |
| 23:05:53 | mriedem | no i'm saying trying to backport the uuid matcher stuff is full of dragons | |
| 23:06:19 | melwitt | ok | |
| 23:06:20 | mriedem | it makes some things simpler though, e.g. your issue where compute failed to restart b/c it couldn't create a new provider with the same name | |
| 23:06:22 | melwitt | noted | |
| 23:06:44 | melwitt | ok, I see. yeah, I think sean-k-mooney mentioned that too but I didn't understand it at the time | |
| 23:06:46 | mriedem | if the uuids are all synced, compute restarts, creates the compute node with the same uuid and finds the provider already exists with that uuid | |
| 23:07:18 | mriedem | not so lucky with libvirt though | |
| 23:07:30 | mriedem | b/c the uuid will be unique per compute node record on the same host | |
| 23:07:46 | mriedem | granted our code that checks to see if the provider already exists could be smarter | |
| 23:08:23 | mriedem | around here https://opendev.org/openstack/nova/src/tag/19.0.0/nova/scheduler/client/report.py#L568 | |
| 23:08:34 | melwitt | ok, hm that last part is interesting because that sounds like our second case (non-ironic) | |
| 23:08:40 | mriedem | if we found a provider with the same name we should probably just use it | |
| 23:09:10 | melwitt | yeah. I think fixing this would kill two birds with one stone | |
| 23:09:21 | melwitt | ironic and non-ironic, unless I'm missing something | |
| 23:09:33 | melwitt | *fixing it in that way | |
| 23:09:37 | mriedem | we can also easily find a provider with the same name using GET /resource_providers?name=<name> | |
| 23:09:43 | mriedem | if we find out, use it | |
| 23:09:46 | mriedem | *one | |
| 23:09:54 | melwitt | aye | |
| 23:09:57 | mriedem | maybe | |
| 23:10:04 | mriedem | i mean there could be dragons there too i'm not thinking about | |
| 23:10:06 | melwitt | that would be an extra call tho | |
| 23:10:21 | mriedem | only if you 409 | |
| 23:10:25 | melwitt | right | |
| 23:10:31 | melwitt | oh | |
| 23:10:33 | mriedem | because clearly we aren't getting this back when we hit this name_conflict = 'Conflicting resource provider name:' | |
| 23:10:35 | melwitt | I yeah | |
| 23:10:58 | mriedem | or maybe we are getting something like that but the message changed in placement, idk | |
| 23:11:02 | mriedem | see the todo from efried | |
| 23:11:42 | melwitt | ok | |
| 23:12:07 | melwitt | so maybe I can try a patch for this and get efried to look for dragons | |
| 23:12:32 | efried | (I haven't been following the conversation, and need to split real soon, but sure, add me to a patch) | |
| 23:12:37 | melwitt | because this looks like it could be a small backportable change that would save us in both the ironic and non-ironic cases | |
| 23:15:09 | melwitt | thanks for the help on this | |
| 23:15:14 | dansmith | I'm not sure how I feel about reusing the provider by name | |
| 23:15:21 | melwitt | ah dammit | |
| 23:15:25 | dansmith | we're kinda making that a primary key because the name has to be unique, | |
| 23:15:33 | dansmith | but if you think about compute nodes in split brain, | |
| 23:15:38 | dansmith | they're going to fight over the same provider | |
| 23:15:48 | dansmith | and with conductor groups in ironic, | |
| 23:16:07 | dansmith | or two ironics and separate computes, | |
| 23:16:18 | dansmith | if you ended up with two nodes of the same name, nova isn't going to know they're different and is going to fight over them | |
| 23:16:29 | dansmith | and by fight, I mean silently overwrite inventory | |
| 23:16:43 | dansmith | if that provider has allocations, things are going to get all messed (the eff) up I think | |
| 23:17:10 | dansmith | moving the ironic node id to be the provider id is the right move (which we've already done) | |
| 23:17:32 | dansmith | so I dunno, maybe that means for <rocky, the name hack works, but... it's kinda contrary to the point of the provider id | |
| 23:17:45 | melwitt | what's the right move in the non-ironic case then? | |
| 23:17:45 | dansmith | melwitt: I think I hinted at this in my email on the internal thread about this | |
| 23:18:01 | dansmith | for the service delete? | |
| 23:18:08 | dansmith | that's probably even worse really, | |
| 23:18:17 | melwitt | I assume it's service delete. I don't know for sure how they got into that state | |
| 23:18:34 | dansmith | because if you end up with two computes with the same name due to some dns or dhcp breakage, they'll take over each others' providers (and allocations) which would be helzabad | |
| 23:18:37 | melwitt | that thread spun off from the ironic bz | |
| 23:19:22 | dansmith | you will have other issues if you have a name clash, obviously, but if you mix allocations from two computes together, or have one go negative because it's a smaller compute, just... hard to debug and fix | |
| 23:19:45 | dansmith | so anyway, I dunno | |
| 23:20:01 | dansmith | not saying don't do it.. glad we don't have to do it on master, but I'm not super confident that it's a great idea for <rocky either | |
| 23:20:02 | melwitt | ok... sigh.. so with the moving node id to be provider id, would that not require a migration step if we were able to backport to queens? | |
| 23:20:17 | dansmith | that only affects ironic | |
| 23:20:50 | melwitt | yeah sorry. going back to the ironic thing again, to fix that one in that way, would it require a migration step? | |
| 23:21:26 | dansmith | as I said on that thread, you'd have to make sure all the computes rolled to that change atomically, and yeah I dunno how the allocations get or got moved with that when we transitioned, | |
| 23:21:35 | dansmith | but not reasonable for a backport either way | |
| 23:21:49 | melwitt | I can and will publish the workaround steps but given the number of customers hitting it, I dunno, thinking about trying the backport | |
| 23:21:52 | dansmith | reversing the order of provider delete works around the ironic issue doesn't it? | |
| 23:22:01 | melwitt | yeah it does | |
| 23:22:25 | dansmith | that's reasonable, backporting the node uuid thing is not reasonable I think | |
| 23:22:38 | melwitt | ok. got it | |
| 23:22:55 | melwitt | thanks | |
| 23:24:15 | melwitt | the non-ironic thing I think need more information because I don't know how it got into the state. and no idea how to workaround bc they can't migrate any instances away from it because all migrations fail with ResourceProviderCreationFailed | |
| 23:24:48 | melwitt | and there's no heal_allocations so they can't delete the allocations and RP and restore allocations | |
| 23:24:58 | dansmith | well, they can | |
| 23:25:02 | dansmith | using osc-placement | |
| 23:25:14 | dansmith | I mean, you can script that for them | |
| 23:25:24 | dansmith | or backport heal allocations, that's much less scary I think | |
| 23:25:43 | melwitt | yeah, that's what I'm thinking | |