| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-11-10 | |||
| 15:23:13 | gibi | stephenfin: if this is an invalid config then nova-compute is allowed to fail on it | |
| 15:23:15 | dansmith | gibi: definitely. we don't deploy it that way in devstack, and I'm pretty sure we've had and fixed this issue downstream | |
| 15:23:25 | dansmith | I'm surprised the upstream tripleo is still doing it | |
| 15:23:38 | stephenfin | Okay, can we add a check to nova-compute startup to hard fail on that config being present? | |
| 15:23:42 | stephenfin | So the error is more obvious? | |
| 15:23:47 | dansmith | sure | |
| 15:24:05 | gibi | stephenfin: that is a good point, but I think the current service level check runs first and fail first | |
| 15:24:23 | gibi | stephenfin: so I cannot add this to the _inithost | |
| 15:24:29 | gibi | or I can but it is pointless | |
| 15:24:44 | gibi | buuut I might be able add someting to the cmd | |
| 15:25:27 | dansmith | so, we have another such check in rpcapi, and a catch for them doing it wrongly, with a warning to explain | |
| 15:26:02 | dansmith | https://github.com/openstack/nova/blob/master/nova/compute/rpcapi.py#L439-L457 | |
| 15:26:25 | dansmith | so, I'm guessing they've also not configured upgrade_levels=auto, | |
| 15:26:29 | dansmith | else they would be exploding there too | |
| 15:26:52 | gibi | could be, my tripleoo knowledge is close to 0 | |
| 15:27:18 | sean-k-mooney | i think we have a down stream bug for this | |
| 15:27:23 | dansmith | point being, this is not a new behavior or requirement | |
| 15:27:28 | sean-k-mooney | owalsh was fixing it | |
| 15:27:42 | dansmith | sean-k-mooney: I thought he did a while ago, which is why I'm surprised we're still seeing it | |
| 15:27:51 | sean-k-mooney | we hilighted that there nova.conf was wrong | |
| 15:27:56 | gibi | dansmith: thanks for the pointer I will add that to my comment in the bugreport asking tripleoo to fix the conf | |
| 15:27:57 | sean-k-mooney | im not sure if it has merged | |
| 15:28:02 | dansmith | gibi: ++ | |
| 15:28:15 | sean-k-mooney | he was rewriting the config gen to not put the db creds in the compute nodes | |
| 15:28:37 | sean-k-mooney | we merged a hack to make 16.0 work but i dont know if the real fix is merged | |
| 15:28:39 | sean-k-mooney | ill check | |
| 15:29:13 | sean-k-mooney | https://review.opendev.org/#/c/718552/ | |
| 15:29:14 | dansmith | gibi: we have another check for api.connection in conductor, which is how it knows if it's a cell or global conductor. doesn't affect compute, but just FYI for prior art | |
| 15:29:21 | sean-k-mooney | that is the ooo fix | |
| 15:29:27 | sean-k-mooney | its not merged yet | |
| 15:29:40 | dansmith | sean-k-mooney: ah yeah, from april.. hence my assuming it was done | |
| 15:29:47 | sean-k-mooney | dansmith: they used to config auto | |
| 15:29:47 | dansmith | 2020 has been a bit of a time warp | |
| 15:29:53 | sean-k-mooney | the hack was to not | |
| 15:29:56 | owalsh | hey... | |
| 15:30:02 | dansmith | sean-k-mooney: ah, heh | |
| 15:30:03 | sean-k-mooney | owalsh: o/ | |
| 15:30:23 | openstack | Launchpad bug 1871482 in tripleo "nova-compute must not configure api database" [High,In progress] - Assigned to Oliver Walsh (owalsh) | |
| 15:30:23 | sean-k-mooney | this is the bug https://bugs.launchpad.net/tripleo/+bug/1871482 | |
| 15:30:46 | sean-k-mooney | this was the hack https://review.opendev.org/#/c/737287/ | |
| 15:30:50 | sean-k-mooney | which is train only | |
| 15:30:58 | sean-k-mooney | not on master | |
| 15:31:07 | sean-k-mooney | that is why tehy are hitting this now | |
| 15:31:15 | sean-k-mooney | victoria and train dont have it | |
| 15:31:35 | sean-k-mooney | *victoria and master | |
| 15:31:37 | owalsh | ok, they need to stop doing that. RPC version pinning is pointless with FFU | |
| 15:32:12 | dansmith | owalsh: note that this new failure is unrelated to pinning, | |
| 15:32:27 | dansmith | but to a check for service versions which fails on the compute if they have api_database.connection set on the compute | |
| 15:32:57 | sean-k-mooney | yep that was the root cause of the orginal issue | |
| 15:33:02 | sean-k-mooney | so its the same thing | |
| 15:33:17 | dansmith | sean-k-mooney: right, they just can't work around this new one by disabling =auto | |
| 15:33:29 | sean-k-mooney | ah ok | |
| 15:33:47 | sean-k-mooney | well i think they only get this check if we have auto set | |
| 15:33:51 | dansmith | no | |
| 15:33:57 | sean-k-mooney | no? | |
| 15:33:57 | dansmith | this is now done at compute startup | |
| 15:34:02 | dansmith | always | |
| 15:34:03 | sean-k-mooney | yes | |
| 15:34:39 | sean-k-mooney | oh instead of condtionally | |
| 15:34:41 | sean-k-mooney | got it | |
| 15:34:53 | owalsh | ok, this is master only? | |
| 15:35:00 | dansmith | the fail is master only, | |
| 15:35:02 | sean-k-mooney | so ya not a nova bug https://review.opendev.org/#/c/718552/ just need to be finished | |
| 15:35:07 | dansmith | we backported it to V but as a warning only | |
| 15:35:23 | gibi | dansmith: I'm affraid the backport also blows | |
| 15:35:28 | sean-k-mooney | we shoudl be backporting the ooo change in anycase | |
| 15:35:31 | dansmith | ohh, right right | |
| 15:35:37 | dansmith | because it tries and explodes before it warns | |
| 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 | gibi | dansmith: good idea | |
| 15:36:28 | sean-k-mooney | well do we need to | |
| 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 | gibi | yes, I want to keep stable stable, or else elod will haunt me | |
| 15:37:21 | sean-k-mooney | yep and it was orginally a 16.1 relase blocker | |
| 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 | 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:40 | stephenfin | I know you already know, but the addition of the abort needs a reno | |
| 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 | |