| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-10-17 | |||
| 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 | noonedeadpunk | but you 100% have them until you restart computes, that will happen only afterwards? | |
| 18:32:44 | sean-k-mooney | and forget to remove the compute service records | |
| 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 | sean-k-mooney | noonedeadpunk: yes it is | |
| 18:36:15 | noonedeadpunk | but tight it to the compute version is indeed a good idea | |
| 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 | |
| 18:53:58 | noonedeadpunk | I mean. I can ignore WARNs and just make them pass.... But what value of warns then... | |
| 18:54:32 | noonedeadpunk | (or depending on the source version for upgrade for 2024.1 | |
| 18:54:52 | sean-k-mooney | noonedeadpunk: no warning are ment to be "this thing is deprecated and you should fix it before upgrading again" | |
| 18:55:00 | dansmith | sean-k-mooney: well, maybe comment on that in the bug and we can get bauzas to opine tomorrow. ISTR he was pro check as well, but maybe only because you said it and it "seems right" :) | |
| 18:55:48 | noonedeadpunk | sean-k-mooney: then it's worth fixing docs to state that | |
| 18:55:52 | sean-k-mooney | if we keep the check we need to filter out ironic compute nodes too and or add the min compute version cehck | |
| 18:56:04 | sean-k-mooney | but sure ill comment on the bug | |
| 18:56:56 | sean-k-mooney | noonedeadpunk: to be honest we try to avoid requiuring the operator to do things to be able to upgrade | |
| 18:56:59 | noonedeadpunk | As right now reading the doc it sounds a bit different then it's fine now - but will be not next time | |
| 18:57:27 | sean-k-mooney | so we very really add nova-status checks becuase we try to amke sure not to design something to require them | |
| 18:58:20 | sean-k-mooney | s/really/rarely/ | |
| 18:58:21 | noonedeadpunk | and you're doing great job to have that said | |
| 18:59:08 | sean-k-mooney | if you look at the first copule they are things like you must have placment, or cells v2 | |
| 18:59:13 | noonedeadpunk | And I'm not insisting that we're doing things right - it's just we're doing them how it's written more or less... And if you say we should do things differently - I'm really fine with that, and can propose update to docs as well | |
| 18:59:21 | sean-k-mooney | and more recently the service user token for the cve | |
| 19:00:28 | noonedeadpunk | Yeah, I guess one "but" for service user token - is that it's not since 24.0.0 as it was during Zed in fact and then backported... But anyway - it's small detail | |
| 19:03:24 | noonedeadpunk | I also wondering how nobody has raised that thing until now... As we've seen failure in CI right after the coordinated release, but nobody had time to look into the rootcause... | |
| 19:05:40 | sean-k-mooney | right so the upgrade check is ment to help catch this before you deploy to production | |
| 19:06:06 | sean-k-mooney | what this si also telling me is we dont actullly fail on error in our grenade jobs. | |
| 19:06:31 | sean-k-mooney | that would have caught this before it merged i think | |
| 19:10:24 | dansmith | well, tbh I think we're running it after upgrade to see if you've got everything done | |
| 19:10:28 | dansmith | and thus it's passing | |
| 19:10:49 | dansmith | it's supposed to be used for that as well.. basically "am I done with my homework" | |
| 19:11:46 | sean-k-mooney | ya that makes sense i gues although this would still fial in the multi node job in that case | |
| 19:12:00 | sean-k-mooney | since we only upgrade the contoler node and not the extra compute | |
| 19:26:10 | noonedeadpunk | Sorry, I think I have one more question. Today I got quite stuid idea, but wanna confirm how stupid it is :) | |
| 19:26:52 | noonedeadpunk | How bad is to run `nova-manage cell_v2 discover_hosts --by-service` if you don't have ironic | |
| 19:27:27 | noonedeadpunk | I see there's a performance penalty of doing that in the docs... | |
| 19:27:46 | noonedeadpunk | But that would simplify some logic quite a lot on the other hand... | |
| 19:30:21 | noonedeadpunk | So wondering how bad that trade off might be from the nova prespective | |
| 19:57:59 | opendevreview | Alexey Stupnikov proposed openstack/nova master: Add functional tests to reproduce bug #1994983 https://review.opendev.org/c/openstack/nova/+/863416 | |
| 19:59:48 | opendevreview | Alexey Stupnikov proposed openstack/nova master: Add functional tests to reproduce bug #1994983 https://review.opendev.org/c/openstack/nova/+/863416 | |
| 19:59:56 | opendevreview | Alexey Stupnikov proposed openstack/nova master: Log some InstanceNotFound exceptions from libvirt https://review.opendev.org/c/openstack/nova/+/863665 | |
| 20:18:24 | colby__ | Hey Guys. We are in the process of upgrading our hypervisors to Yoga. We are on centos8 stream. Once I update openstack and qemu/libvirt packages instances are not able to be live migrated to the hypervisor. Im seeing the following: | |
| 20:18:26 | colby__ | qemu-kvm: Missing section footer for 0000:00:01.3/piix4_pm#0122023-10-17T19:07:20.896349Z qemu-kvm: load of migration failed: Invalid argument | |
| 20:19:09 | colby__ | did the commands to migrate change at all with yoga release? Where can I see that in the code? Im wondering if this is a qemu update issue | |
| 20:50:14 | opendevreview | Merged openstack/nova master: Install lxml before we need it in post-run https://review.opendev.org/c/openstack/nova/+/898435 | |