| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-10-17 | |||
| 18:19:19 | noonedeadpunk | ahaa..... that what can be wrong | |
| 18:19:34 | noonedeadpunk | as I'm running it with 2023.1 code before restart | |
| 18:19:39 | noonedeadpunk | sorry | |
| 18:19:43 | noonedeadpunk | 2023.2 code | |
| 18:20:12 | dansmith | sean-k-mooney: running the new nova-status before you upgrade right? | |
| 18:20:17 | noonedeadpunk | so 1. upgrade code. 2. run check 3. restart services if it passes rollback if it's not | |
| 18:20:29 | dansmith | so we shouldn't have made that an error because without having run computes, you can't have had the uuids added | |
| 18:20:31 | noonedeadpunk | (it's current flow) | |
| 18:20:41 | sean-k-mooney | so to upgrade to 2023.2 form 2023.1 you would run the 2023.2 version of nova-status before doign the upgrde | |
| 18:21:13 | noonedeadpunk | yeah, I think that what I do actually | |
| 18:21:36 | noonedeadpunk | Like code is upgraded but services still running old code | |
| 18:22:30 | sean-k-mooney | dansmith: am i right about runnign the 2023.2 nova status when upgradeign to 2023.2 or should it be the 2023.1 version | |
| 18:22:35 | sean-k-mooney | i feel like im wrong about htat | |
| 18:23:12 | sean-k-mooney | we are ment to deprecate thign at least one release early so we normally put the status check in with the deprecation as a warning | |
| 18:23:17 | dansmith | honestly, I think it's supposed to be "am I done with my upgrade yet" | |
| 18:23:21 | noonedeadpunk | but how with old code you can be aware if it's safe to upgrade to the new one? | |
| 18:23:28 | dansmith | which before you upgrade will be "no" | |
| 18:23:45 | dansmith | but maybe error should only be raised if you have to do something before upgrading, idk | |
| 18:24:03 | dansmith | I wrote that at your request (IIRC) and don't recall any review comments about it :) | |
| 18:24:16 | sean-k-mooney | :) | |
| 18:25:31 | sean-k-mooney | so the currnt check woudl only pass after all compute are fully upgraded but it also should only be an error if th min compute service version is above the version where we stated doing that | |
| 18:26:19 | sean-k-mooney | noonedeadpunk: is this curretly blocking you form upgrading by the way | |
| 18:26:31 | sean-k-mooney | or were you asking how to make the check happy | |
| 18:26:47 | sean-k-mooney | if the later then you need to start the computes with the bobcat code | |
| 18:26:54 | sean-k-mooney | and it should pass after they have all started | |
| 18:26:55 | noonedeadpunk | Well. It makes our upgrade code fail in osa | |
| 18:27:17 | noonedeadpunk | And I'm looking how to make our upgrade jobs happy as well as users that would perform upgrade | |
| 18:27:43 | sean-k-mooney | i would need to check our docs for which version of nova status we expect to be run and where | |
| 18:28:15 | noonedeadpunk | It says nothing after Xena.... | |
| 18:28:28 | noonedeadpunk | if you're about https://docs.openstack.org/nova/latest/cli/nova-status.html#upgrade | |
| 18:28:44 | sean-k-mooney | nova-status on -1 tells you about the thigns that are deprecated that we know might brake you in the future | |
| 18:28:57 | sean-k-mooney | so before upgrade you shoudl fix those with the n-1 version | |
| 18:29:09 | sean-k-mooney | then do the upgrade. | |
| 18:29:57 | noonedeadpunk | mhm. Ok, I see, so it should run not on the upgraded code | |
| 18:29:58 | sean-k-mooney | ok so reading the doc text | |
| 18:30:12 | sean-k-mooney | its say "Performs a release-specific readiness check before restarting services with new code." | |
| 18:30:39 | sean-k-mooney | that would implcy that the new check should be a warning in 2023.2 and an error in 2024.1 i think | |
| 18:30:44 | noonedeadpunk | Well, I read it in a way - upgrade code, run test, restart services | |
| 18:30:52 | dansmith | sean-k-mooney: yep | |
| 18:31:06 | sean-k-mooney | although with slurp it shold be a warnignin 2024.1 as well? | |
| 18:31:15 | noonedeadpunk | I'm not sure it should be a warning even.... | |
| 18:31:28 | noonedeadpunk | and yeah, error only in 2024.2 | |
| 18:31:55 | noonedeadpunk | As that is really expected, that you won't have that at this stage, so what to warn about... | |
| 18:32:21 | sean-k-mooney | well it may mean you have old non upgraded compute service records | |
| 18:32:37 | sean-k-mooney | that is pretty common where peope scale in or remove say ironic | |
| 18:32:44 | sean-k-mooney | and forget to remove the compute service records | |
| 18:32:44 | noonedeadpunk | but you 100% have them until you restart computes, that will happen only afterwards? | |
| 18:32:53 | dansmith | noonedeadpunk: it needs to go from some state to "all good" after you're done with the upgade | |
| 18:32:59 | sean-k-mooney | that hsoudl be caust by the "Check: Older than N-1 computes " check | |
| 18:33:06 | dansmith | I don't think slurp has anything to do with this | |
| 18:33:34 | noonedeadpunk | well, it does, as otherwise you can't jump 2023.1 -> 2024.1? | |
| 18:33:45 | noonedeadpunk | as you have to do 2023.2 regardless ? | |
| 18:34:05 | noonedeadpunk | Or I'm totally lost why slurp even a thing... | |
| 18:34:32 | sean-k-mooney | so i think given where this is run in the upgrade it would only be valid to be an error if the min compute service version was above that in 2023.2 | |
| 18:34:43 | dansmith | slurp has nothing to do with it because it shouldn't be erroring out before you'e done whatever upgrade will result in the new ids | |
| 18:35:22 | dansmith | sean-k-mooney: I think we could move it to an error and version check before we start relying on these, but no need to make it an error right now I think | |
| 18:35:31 | noonedeadpunk | well, docs for the command say, that it should be run BEFORE service restart. So you can't be done with upgrade, it's pre-upgrade check basically | |
| 18:35:38 | sean-k-mooney | dansmith: i agree | |
| 18:36:15 | noonedeadpunk | but tight it to the compute version is indeed a good idea | |
| 18:36:15 | sean-k-mooney | noonedeadpunk: yes it is | |
| 18:36:36 | sean-k-mooney | the command was intened to tell you before you upgrade that "x will break because your forgot to do something" | |
| 18:36:41 | noonedeadpunk | as if it's n-2 for 2024.1 -> warning, n-1 - error | |
| 18:37:13 | sean-k-mooney | noonedeadpunk: so 2024.1 will be the first release to fully supprot n-2 | |
| 18:37:15 | noonedeadpunk | yeah, but well, if the feature has appeared afterwards - you really can't do that before? | |
| 18:37:16 | dansmith | noonedeadpunk: can you file a bug so we can backport? | |
| 18:37:22 | noonedeadpunk | ++ | |
| 18:38:05 | sean-k-mooney | dansmith: we just need to change this to warnign correct https://github.com/openstack/nova/blob/master/nova/cmd/status.py#L291-L302 | |
| 18:38:20 | dansmith | doing it now | |
| 18:38:32 | noonedeadpunk | just to state this one more time - warning will fire in 100% cases, right? | |
| 18:39:04 | sean-k-mooney | hum yes until the upgrade is compelte | |
| 18:39:09 | dansmith | yes | |
| 18:39:34 | noonedeadpunk | And what docs say - `At least one check encountered an issue and requires further investigation.` So all who do upgrades will spend some time to find out that they should proceed first when it's not inclined but the command nature? | |
| 18:39:43 | dansmith | it wouldn't make much sense to say "you pass this test with OK" and then after you restart it goes to warning or error | |
| 18:39:56 | dansmith | if we had a way to skip a test because it's irrelevant, then we'd do that, otherwise I think warning makes sense | |
| 18:39:58 | noonedeadpunk | it does on 2024.1 | |
| 18:40:05 | noonedeadpunk | if you upgrade from 2023.2 | |
| 18:40:44 | sean-k-mooney | dansmith: if we modify the check to do a min compute version check it might but then perhasp i was wrong to ask for this as its not as useful as i tought | |
| 18:40:45 | dansmith | does what? make sense? I disagree | |
| 18:41:16 | sean-k-mooney | noonedeadpunk: baislly i asked for this to catch the case where you are upgrading aging and had not started the computed i.e. to 2024.2 i guess | |
| 18:41:33 | noonedeadpunk | but then I'm really not getting what are expectations on me as operator to perform upgrade from 2023.1 to 2023.2 and what I should do when I see that warning | |
| 18:42:26 | dansmith | saying we checked a thing and everything is good when we really checked and see that you're not but we're ignoring the done-ness of it because you're in the middle of an upgrade doesn't seem to fit very well to me | |
| 18:42:36 | dansmith | like I say if we had "skipped" or "not due yet" then that'd make sense | |
| 18:42:37 | noonedeadpunk | yeah, I kind of already got what it does and why exist :) just trying to say that raise warning for expected behaviour is also... meh? | |
| 18:45:09 | sean-k-mooney | i wonder if it would be beter to remove this check. i know i asked for it orginally but im wondering if its adding benifit. im also wondering if there is another bug | |
| 18:46:07 | sean-k-mooney | we are doing " cn_no_service = main_db_api.compute_nodes_get_by_service_id(ctx, None) | |
| 18:46:12 | sean-k-mooney | " | |
| 18:46:38 | sean-k-mooney | so cn_no_service is the list of all compute nodes that have service id set to None | |
| 18:46:46 | dansmith | sean-k-mooney: it's really more applicable to the release where we start requiring it | |
| 18:47:11 | sean-k-mooney | dansmith: ya it is. im wonderign what happens for ironic in this case | |
| 18:47:59 | dansmith | idk. I have a big "if ironic: return" in my head over all that stuff :P | |
| 18:48:47 | sean-k-mooney | i think this will always fail if you have ironic | |
| 18:48:57 | sean-k-mooney | but i am also pretty sure we have not test coverage | |
| 18:49:00 | noonedeadpunk | https://bugs.launchpad.net/nova/+bug/2039597 | |
| 18:50:02 | sean-k-mooney | as in, in the compute manager i think your correect we have "if ironic: return" so i dont think we set the service id for ironic computes | |
| 18:50:35 | dansmith | sean-k-mooney: no I meant I have that conditional in my _brain_ :) | |
| 18:50:45 | sean-k-mooney | oh hehe | |
| 18:51:29 | dansmith | okay well, I have the FAIL->WARN thing queued, I can either submit that or change to a revert | |
| 18:53:11 | sean-k-mooney | given this converstaion im inclided to revert | |
| 18:53:37 | sean-k-mooney | and then when we actully start depenign on this we can decide if we shoudl add a status check | |
| 18:53:47 | sean-k-mooney | as you suggested before | |