Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-11-10
15:35:42 gibi yes
15:35:49 dansmith gibi: we could catch DBNotAllowed and warn on stable
15:35:55 gibi dansmith: yes, I will do that
15:36:11 dansmith gibi: maybe just do that on master too, to satisfy stephenfin's concern and then we can backport it cleanly
15:36:28 sean-k-mooney well do we need to
15:36:28 gibi dansmith: good idea
15:36:33 gibi thanks
15:36:39 sean-k-mooney we can but we shoudl be fixing ooo
15:36:49 sean-k-mooney they are genering an invalid config
15:36:55 dansmith sean-k-mooney: we're just talking about logging more helpful messages,
15:36:59 dansmith and avoiding a breakage on stable
15:37:04 dansmith but yes, this is a ooo thing to fix
15:37:21 sean-k-mooney yep and it was orginally a 16.1 relase blocker
15:37:21 gibi yes, I want to keep stable stable, or else elod will haunt me
15:37:38 dansmith sean-k-mooney: we backported this as a warning, but we'll explode before we warn now, if they have this config, so we need to also catch this error and warn for that too to avoid introducing a new startup fail on stable
15:37:54 gibi ^^++
15:38:10 sean-k-mooney sure that a good thing to do
15:38:31 sean-k-mooney im just going to find the downstream bz and bump it with a blocker flag request before 16.2
15:39:19 dansmith yup
15:39:59 owalsh ack, I think https://review.opendev.org/718552 is pretty much done but I wanted to run some manual tests for corner cases before removing the -W
15:42:48 stephenfin Going with a warning rather than exploding on master would be less aggressive, for sure
15:43:28 dansmith we still need to abort startup on master,
15:43:36 stephenfin yup, agreed
15:43:41 dansmith we can just catch the DB error, and log something relevant before we then abort
15:43:48 gibi yes
15:44:14 stephenfin gotcha
15:44:40 stephenfin I know you already know, but the addition of the abort needs a reno
15:44:40 dansmith and since the error is compute specific, we can do that in utils and know that it will only happen on computes, for the log
15:44:58 dansmith gibi already added one yeah?
15:45:24 dansmith https://review.opendev.org/#/c/761923/1/releasenotes/notes/warn-when-services-started-with-old-compute-fc80b4ff58a2aaea.yaml
15:45:47 dansmith this is going to abort for the same reason
15:46:05 gibi yepp, reno is there
15:46:18 gibi it is a bit different reno on the master
15:46:21 stephenfin I thought it would abort because we had API DB configuration on a compute node?
15:46:30 gibi this is the master one https://review.opendev.org/#/c/738482/10/releasenotes/notes/prevent-starting-services-with-old-compute-fc80b4ff58a2aaea.yaml
15:46:55 bauzas dansmith: just a quick question, amirite to delete https://review.opendev.org/#/c/761452/1/nova/compute/manager.py@a5115 for the RPC API 6.0 ?
15:47:35 gibi stephenfin: I can extend the reno to explain the api config issue too
15:47:36 dansmith gibi: you could catch the db exception and then do the single-cell check if you get it,
15:47:54 gibi dansmith: that is a good point too
15:47:57 dansmith gibi: avoiding the different behavior, although that won't help get people to stop configuring their computes wrong
15:48:13 stephenfin gibi: I think that would be good
15:48:15 dansmith so I guess decide whether you want to lump that in or not
15:48:33 gibi I think if the rule is not to configure api db for the compute service, then it is clearer to fail on it
15:48:40 bauzas dansmith: because even if the wallaby computes would use 5.11 (as it's the victoria API version) in case of a rolling upgrade, then they will only get objects ?
15:49:11 dansmith gibi: so, you could make this not explode, and then follow up with a separate check of just that config thing with a separate abort, separate reno, etc.. just saying you _can_ separate them I think, if you want
15:49:17 sean-k-mooney gibi: you should not configre any db creds on teh compute not just api
15:49:30 dansmith sean-k-mooney: indeed
15:49:37 sean-k-mooney since we removed the local conductor in icehouose they have not been used
15:49:45 gibi dansmith: ahh, so just split the change into two commit
15:49:47 sean-k-mooney *icehouse ish
15:49:50 gibi dansmith: I can do that
15:50:12 dansmith gibi: if you want.. make the existing check smart if it hits the compute error, knowing it should fall back to the single-cell check,
15:50:21 dansmith gibi: and then a separate patch to check that no db creds are set
15:50:27 dansmith gibi: I'm happy to write that second one if you want
15:50:51 gibi dansmith: thanks, I think I can do both
15:50:55 gibi and I will make them separate
15:50:59 stephenfin so in one patch, we're catching that error and logging a warning instead (or falling back to the second call), and then in another we hard fail if DB configuration is present on a compute node?
15:50:59 dansmith gibi: okay
15:51:05 stephenfin maybe I should just wait for the patches :)
15:51:15 dansmith stephenfin: yes
15:51:20 stephenfin lovely :-)
15:51:44 gibi :)
15:52:07 dansmith bauzas: you're asking if you can effectively only support 5.11 and not 5.0 in the bridge release that also supports 6.0?
15:52:35 bauzas dansmith: yeah, or whether we could wait until X
15:52:43 bauzas tbh, we would need to wait
15:53:21 sean-k-mooney bauzas: well 6.0 would be 5.11 right
15:53:22 dansmith bauzas: I think technically that works, but it's cheating of course... I guess I'd prefer to wait just to make it fully clean,
15:53:22 bauzas but given the 5.1 version was for something like Queens, then I'm pretty sure that none of the clients would pass a legacy dict
15:53:30 sean-k-mooney that what we normally do
15:53:33 dansmith but if it's something very hard to hold across the line, then maybe worth it
15:53:41 bauzas dansmith: okay, then I'll remove it
15:53:48 bauzas and we would remove those for X
15:54:06 sean-k-mooney dansmith: isnt that what we did for the 5.0 and 4.0 bumps
15:54:10 bauzas in case people look at my change later, maybe they wouldn't understand why we would remove this
15:54:27 bauzas sean-k-mooney: we still need to support 5.x in Wallaby
15:54:32 bauzas at least 5.11
15:54:43 dansmith bauzas: I'm not sure what you've decided.. are you going to leave the 5.0 stuff until X or strip it down to only be 5.11?
15:54:59 bauzas dansmith: tbh, I don't have a opinion
15:55:14 sean-k-mooney i think we stiped it for 5.0
15:55:16 bauzas dansmith: but after what you told, let's not delete the 5.0 support in my change
15:55:23 sean-k-mooney we could jsut check the patch
15:55:36 dansmith if you do it now, then you've got a lot of change, right? de-5.0ing the existing code, supporting 6.0, plus anything else we add between now and then, followed by a de-5.11ing in X
15:55:43 bauzas we could discuss about whether we would delete the <5.11 support before X
15:55:45 dansmith whereas if you leave it all, then you just de-5.x the whole thing at once
15:55:51 bauzas dansmith: yzah
15:55:55 bauzas anyway, nevermind
15:56:03 bauzas I'll return it
15:56:42 dansmith sean-k-mooney: I don't think I stripped the older 4.x support when we added 5.0 until after we dropped 4.x, but I could be wrong
15:56:46 sean-k-mooney this was the 5.0 bump https://github.com/openstack/nova/commit/a761e57368280b6d3e931831ecd393fd5787b3ef#diff-91f79786d7e3744c39926c88bbafe3b727630fa4eb48e845686d7f12f876d067
15:57:12 bauzas sean-k-mooney: dansmith: nevermind, again
15:57:19 sean-k-mooney so it loks like queens supported both
15:57:21 dansmith sean-k-mooney: that's the drop of 4.x not the bump
15:57:24 sean-k-mooney then we droped in rocky
15:57:26 dansmith right
15:57:33 bauzas for the main change, I'll just provide a new 6.0 RPC version but we will support 5.0
15:57:34 sean-k-mooney so i guess we do that
15:57:41 sean-k-mooney so drop in X
15:58:04 dansmith and you notice all the 4.x intermediate version parameter defaults are still there
15:58:05 bauzas and then we could do a remove in a later change that we could drop for 5.0 to 5.10, or just wait for X
15:58:16 dansmith bauzas: wait for X IMHO

Earlier   Later