WIP: CNV-62665: fix search by CPU and Memory for stopped InstanceType VMs #2706
Conversation
|
@adamviktora: This pull request references CNV-62665 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.20.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adamviktora The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
| tempMap[namespace] = {}; | ||
| } | ||
|
|
||
| const url = getInstanceTypeExpandSpecURL(vm); |
There was a problem hiding this comment.
take a look if it's possible to reuse:
useK8sWatchResourceshook provided by the plugin API
There was a problem hiding this comment.
-
I think the useInstanceTypeExpandSpec hook can't be used, it can be used only per single VM, the only part I used from that is the url to fetch.
-
If there would be a way to use
useK8sWatchResourceson the expandedV1VirtualMachinethat would be ideal. So have some options of theWatchK8sResourcetype that would return objects that can be fetched from this url:
`api/kubernetes/apis/subresources.${VirtualMachineModel.apiGroup}/${
VirtualMachineModel.apiVersion
}/namespaces/${getNamespace(vm)}/${VirtualMachineModel.plural}/${getName(vm)}/expand-spec`
There was a problem hiding this comment.
sadly you are right - the expand-spec is not a separate resource and useInstanceTypeExpandSpec is probably to heavy to be used for group of VMs. However this reveals the true problem - scalability. Will continue that thread at top level.
rszwajko
left a comment
rszwajko
left a comment
There was a problem hiding this comment.
We might hit performance problems when building vmExpandSpecMap. The worst case scenario is that all VMs need to be expanded - so we get 1 request per VM. Clearly we need API from the backend to get that information in one request.
|
I've written up https://issues.redhat.com/browse/CNV-63726 for the backend to provide this in the status of all VMs. |
adamviktora
changed the title
4 participants
📝 Description
Fixes advanced search by CPU and Memory for stopped InstanceType VMs.
I don't like the fact that the
useVMExpandSpecMaphook is refetching everything whenever the vms array change, if you have any ideas how to make it more efficient, yet simple, let me know.🎥 Demo
Before:
Screen.Recording.2025-05-28.at.16.47.43.mov
After:
Screen.Recording.2025-05-28.at.16.41.19.mov