| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-21 | |||
| 22:37:29 | mriedem | i would guess (1) yes they should be able to differ and (2) assume N-1 | |
| 22:37:48 | mriedem | that's part of the idea behind passing os-vif negotiated objects around so you can do rolling upgrades of those | |
| 22:37:57 | mriedem | over the rest api i mean | |
| 22:38:07 | efried | which one gets to be -1? or are they both allowed to be? | |
| 22:38:16 | efried | ugh, am I making sense? | |
| 22:38:24 | mriedem | i know what you're asking, but i don't have a good answer | |
| 22:38:29 | efried | k | |
| 22:38:29 | efried | well | |
| 22:38:38 | mriedem | i doubt that level of upgrade granularity is documented or tested anywhere | |
| 22:39:17 | mriedem | nova and neutron and cinder and keystone etc should all be able to work with each other at wildly different versions but we don't test upgrades that way | |
| 22:39:29 | mriedem | not because we can't | |
| 22:39:35 | efried | any neutron that does bw as currently written knows that it will work properly with `gethostname()` | |
| 22:39:35 | efried | and | |
| 22:39:35 | efried | it can condition the "new" thing simply on whether nova is exposing the microversion providing the new os-hypervisors qparam | |
| 22:40:05 | efried | try: | |
| 22:40:05 | efried | new thing | |
| 22:40:05 | efried | except NoSuchMicroversion: | |
| 22:40:05 | efried | old thing | |
| 22:40:30 | efried | unless neutron does better discovery than that (which it should be able to, but that doesn't mean it does) | |
| 22:40:34 | efried | Okay, | |
| 22:40:50 | mriedem | i would probably start with just a simple workaround optoin in neutron which is backportable without the microversion stuff | |
| 22:41:00 | mriedem | and the way to deprecate the workaround optoin in neutron is the microversion in nova when it's available | |
| 22:41:03 | efried | why is a workaround necessary? | |
| 22:41:17 | mriedem | because neutron has to do a thing based on how nova CONF.host is set yeah? | |
| 22:41:20 | efried | no | |
| 22:41:27 | efried | CONF.host is only ever right by chance | |
| 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 | |