| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-21 | |||
| 22:41:33 | efried | `gethostname()` is always right | |
| 22:41:49 | efried | ...for the cases it cares about in existing code, which is what we care about for backportability. | |
| 22:42:28 | efried | Anyway, since gibi and sean-k-mooney aren't here right now, and I don't know whether there's a bug yet, I guess I'll throw out a ML post summarizing the discussion and let it fester from there. | |
| 22:46:11 | mriedem | ok | |
| 22:47:27 | melwitt | speaking of downstream bugs, | |
| 22:48:29 | melwitt | we hit a problem downstream where compute node orphan removal was happening and destroyed the compute node record but failed to delete the RP bc keystone or placement was down, | |
| 22:49:06 | melwitt | and after that, nova-compute could not start up again bc it was trying to create compute node record and then failed with 409 dupe from placement when trying to create the same provider | |
| 22:50:07 | mriedem | unrelated, i just wanted to say this makes us look dumb https://docs.openstack.org/api-guide/compute/general_info.html#relationship-with-volume-api | |
| 22:50:14 | melwitt | searching for ResourceProviderCreationFailed led me to mriedem's patch https://review.opendev.org/#/c/678100/1/nova/compute/manager.py@8332 where he posed the question, should we swap the destroy and RP delete ordering, | |
| 22:50:17 | mriedem | and this https://docs.openstack.org/api-guide/compute/server_concepts.html#server-creation | |
| 22:50:23 | melwitt | and I think the answer is yes | |
| 22:51:19 | melwitt | that's odd. I'm not why someone added empty docs. and I hope it wasn't me | |
| 22:51:23 | melwitt | *not sure | |
| 22:51:51 | mriedem | the api-guide was imported i think so no i'm not saying you, i just was looking for some stuff and noticed these | |
| 22:52:04 | mriedem | these giant todo gaps in our user-facing docs are embarrassing | |
| 22:52:11 | mriedem | i'd rather we just delete them than leave them | |
| 22:52:20 | melwitt | yeah, I was gonna say, just remove em | |
| 22:52:36 | melwitt | that was a joke, sometimes I see a thing and be like, wtf who did this and find it was me | |
| 22:54:25 | mriedem | your provider issue is also because in queens we didn't link the ironic node id to the compute node uuid to the provider uuid, we started that in rocky | |
| 22:54:37 | mriedem | so your recourse in queens is deleting the old providers so compute on restart can re-create them | |
| 22:54:40 | mriedem | but you'll have to heal allocations | |
| 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 | |