| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-09-04 | |||
| 13:53:21 | bauzas | something that could then be backportable to Epoxy if we can | |
| 13:53:22 | dansmith | I agree we can't just stop doing it, and making it only present if you don't list, or don't list enough is also not great | |
| 13:53:51 | bauzas | if we can't backport it, then 2.88 will have the same issue | |
| 13:54:23 | sean-k-mooney | the hypervior api is already doing an sqlquery to the compute nodes db table and geting the stats | |
| 13:54:45 | sean-k-mooney | we can put it in that json bob which is entirly unversioned today | |
| 13:54:55 | sean-k-mooney | so if we want to backprot it we can | |
| 13:55:10 | dansmith | yeah was just going to say, ComputeNode has several places we could stash it I think | |
| 13:55:41 | dansmith | but, it will be absent during an upgrade so we probably need to fall back to the old behavior for nodes that aren't upgraded | |
| 13:55:59 | sean-k-mooney | the bit im currntly checking is it purly the time or are we includign the load info too | |
| 13:56:32 | sean-k-mooney | im hoping it just the former but some of our docs implies the latter | |
| 13:57:47 | dansmith | doesn't much matter | |
| 13:58:00 | sean-k-mooney | well its just changes what we need to stash | |
| 13:58:14 | sean-k-mooney | but ya it does not matther in the long run | |
| 13:58:20 | sean-k-mooney | the depercated api https://docs.openstack.org/api-ref/compute/#show-hypervisor-uptime-deprecated | |
| 13:58:22 | bauzas | if we can stuff the uptime value somewhere, sure | |
| 13:58:28 | sean-k-mooney | was the rwaw output of uptime | |
| 13:58:41 | bauzas | without needing to have a new DB field, cool | |
| 13:58:41 | sean-k-mooney | but i think in 2.88 its just the time in second | |
| 13:59:01 | sean-k-mooney | but i just need to check that on a master devstack to confirm | |
| 13:59:01 | bauzas | sean-k-mooney: my concern is not with hypervisor show, again. | |
| 13:59:12 | sean-k-mooney | bauzas: yep i know | |
| 13:59:19 | bauzas | if we continue to call the OS for hypervisor show, meh | |
| 13:59:47 | bauzas | even if that's bad, that's not really an issue | |
| 14:00:18 | bauzas | but, if we are able to persist some uptime value for detailed-list, then of course we could modify hypervisor show | |
| 14:01:08 | jkulik | I don't see anything parsing the `uptime` command output, so it seems to be indeed the full one | |
| 14:01:17 | opendevreview | ribaudr proposed openstack/nova stable/2024.2: Reproducer for bug 2114951 https://review.opendev.org/c/openstack/nova/+/959525 | |
| 14:01:18 | opendevreview | ribaudr proposed openstack/nova stable/2024.2: Fix bug 2114951 https://review.opendev.org/c/openstack/nova/+/959526 | |
| 14:01:39 | sean-k-mooney | it is yes i just confirmed that | |
| 14:02:17 | sean-k-mooney | https://paste.opendev.org/show/bJ59n0G92NoP06hDsfAu/ | |
| 14:02:38 | jkulik | including a newline? :D | |
| 14:02:54 | sean-k-mooney | so i cant jsut retirn the tiem btu i can just stash the value in the stats and skip the rpc call if its there | |
| 14:03:06 | sean-k-mooney | and ya it also has a leadign space | |
| 14:03:43 | dansmith | can't what? | |
| 14:04:09 | sean-k-mooney | sorry, i cant just retive the time since boot and return that value in seconds | |
| 14:04:18 | bauzas | dansmith: given we return the whole uptime, we can't just calculate it | |
| 14:04:20 | sean-k-mooney | dansmith: jkulik suggeted doing that to aovid calling the uptime command | |
| 14:04:46 | sean-k-mooney | in a futre microverion we can jsut not in the backportabel fix | |
| 14:04:48 | dansmith | ack, sorry I just wasn't getting that meaning from the translation | |
| 14:05:02 | bauzas | this is honestly super weird to have that in our API | |
| 14:05:23 | sean-k-mooney | ya that was a pebkac, my keyboard was not aligent so i was typing off center | |
| 14:05:26 | bauzas | I don't disagree with sean-k-mooney when he said we should have stopped to provide it | |
| 14:05:53 | bauzas | but now we have it in detailed-list instead of a specific API call... so that's even worst | |
| 14:06:11 | dansmith | yep, just got to fix it, not the end of the world | |
| 14:08:44 | gibi | ^^ +1 | |
| 14:09:51 | bauzas | dansmith: using a periodic that would cache some raw value ? | |
| 14:10:33 | dansmith | bauzas: just use whatever periodic we use to update the metrics and/or RT IMHO | |
| 14:12:04 | bauzas | I guess we would need to modify the API docs to say that the uptime value can be a bit stale | |
| 14:12:57 | dansmith | does the api guarantee or suggest that it's live? | |
| 14:13:28 | jkulik | with the call taking 40s, I guess it wasn't live already for a lot of them :D | |
| 14:13:39 | dansmith | jkulik: excellent point :D | |
| 14:19:26 | bauzas | dansmith: good point, /me looks at the existing docs | |
| 14:21:35 | bauzas | well, nothing is specifically said, so I guess we can just fix it silently | |
| 14:21:44 | dansmith | :) | |
| 14:23:12 | bauzas | "stats" can be a good CN object field candidate for persisting the value | |
| 14:23:22 | bauzas | that's a dict of strings | |
| 14:23:31 | bauzas | sean-k-mooney: ^ | |
| 14:24:33 | dansmith | IIRC both stats and metrics are actually used for something, but I don't remember all the details | |
| 14:25:11 | opendevreview | Rajesh Tailor proposed openstack/nova stable/2024.2: Fix case sensitive comparison https://review.opendev.org/c/openstack/nova/+/959534 | |
| 14:27:29 | bauzas | dansmith: I just remember now where stats is called | |
| 14:27:49 | sean-k-mooney | bauzas: you realise i litrally said that many many times | |
| 14:27:59 | bauzas | first good news, that's a periodically updated field using the RT update_avail_resources() | |
| 14:28:55 | sean-k-mooney | dansmith: the api refe just say "The total uptime of the hypervisor and information about average load." | |
| 14:29:02 | bauzas | sean-k-mooney: well, then I missed the many times when you said that | |
| 14:29:11 | sean-k-mooney | and everythin else is cached at the period of the update_aviable_resouces perodic | |
| 14:30:01 | dansmith | sean-k-mooney: yeah, it's unfortunate that's in our api ref at all, especially since all that is very os-specific | |
| 14:31:22 | bauzas | anyway, sounds like we have a plan | |
| 14:31:33 | sean-k-mooney | that what the deprecate api says and thsi is what the hypervior deail api say ```The total uptime of the hypervisor and information about average load. Only reported for active hosts where the virt driver supports this feature``` | |
| 14:31:40 | bauzas | we can fix 2.88 and drop that field in a later microversion | |
| 14:32:07 | bauzas | if ops complain about that missing field, then we can tell them to use whatever tool for calling the OS by themselves | |
| 14:32:19 | sean-k-mooney | bauzas: just for extra context gibi and i talk about this a few weeks ago and before that i talk to others about this too. | |
| 14:32:22 | bauzas | ansible, prometheus or nagios, I don't care | |
| 14:32:36 | sean-k-mooney | so this is not the first converation we have had upstream or downstream on how to fix this | |
| 14:32:37 | bauzas | few weeks ago, I was on PTO :) | |
| 14:32:42 | sean-k-mooney | you were on pto for the prior ones | |
| 14:33:12 | bauzas | so I had no context, but the good thing is that we went to the same solution, isn't that good ? | |
| 14:33:16 | sean-k-mooney | yes im aware but we were not activly fixing it because of time pressuer for FF | |
| 14:33:29 | bauzas | I think this is a reasonable bugfix for RC1 | |
| 14:33:54 | bauzas | not RC2 of course as that's a not a Flamingo regression per say | |
| 14:34:00 | bauzas | but I know you know | |
| 14:34:01 | dansmith | bauzas: ++ for rc1 | |
| 14:34:03 | sean-k-mooney | maybe ill see if i can fix it tomrrwo or at least ahve a draft for review | |
| 14:34:07 | dansmith | if it's reasonable to do so | |
| 14:34:43 | bauzas | extending the Stats class by stuffing a new keypair sounds a reasonable solution | |
| 14:35:05 | bauzas | but sean already found the solution, I leave him tell whether he's OK | |
| 14:36:00 | sean-k-mooney | lets just review the patch and we can backprot as normal. if its ready for rc1 cool if not we chan cherry pick after we release 2025.2 | |
| 14:38:01 | dansmith | sean-k-mooney: there's no patch up to review yet, right? just one in your head? | |
| 14:38:21 | sean-k-mooney | exactly im still chekcing if any other virtdriver ever supproted this rpc | |
| 14:39:28 | sean-k-mooney | basicly im not sure i want to stash a sentenal value in the stats for other virt driver | |
| 14:40:08 | sean-k-mooney | if i can show its only libvirt by inspecting the code i can skip that and gard the rpc call behidn both the virt type adn the absence of the key in the dict | |
| 14:40:37 | sean-k-mooney | that way i can aovid stashign garbage values for ironic or vmeare compute nodes | |
| 14:45:33 | sean-k-mooney | looks like zvm also suports it | |
| 14:46:58 | sean-k-mooney | although if your using zvm the contet of the respocne i think is diffent based on the unit tests | |
| 14:48:51 | sean-k-mooney | so zvm and libvirt the rest all raise not implemtned ok that good to know | |
| 14:49:13 | sean-k-mooney | i will probaly put the zvm fix in a spereate commit but it wil be effectivly the same | |
| 14:51:12 | dansmith | super expensive to make that rpc call for every hypervisor only to have them raise NotImplemented :/ | |
| 14:53:05 | sean-k-mooney | yep that why i dont want to even try :) | |
| 14:54:41 | bauzas | stats are already provided by the virt driver and we stuff them in the Stats object using update_stats() | |
| 14:55:03 | bauzas | so we can just return the uptime as part of the virt resources | |
| 14:55:38 | sean-k-mooney | yep thats the plan | |
| 14:56:03 | sean-k-mooney | but the upstream is ment to be the uptime of the node which is drivder depenent. its not the uptime of the comptue service | |