Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KubeClusterMetrics: node and app cpu/mem usage. #72

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

andrewd-zededa
Copy link
Contributor

For kubevirt-eve images this adds per node and eve-user-app cpu/memory usage metrics.

Fix: add cluster_id to ZInfoKubeClusterUpdateStatus

@andrewd-zededa andrewd-zededa force-pushed the kubeclustermetrics branch 2 times, most recently from eaef607 to 4cbe160 Compare November 5, 2024 15:54
}

// Resources used as reported by kubernetes api for pods running eve user apps.
message KubeEVEAppPodMetrics {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only pod metrics ? What about the VMI ? Do we still use pod metrics for VMI too ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each vmi app has an associated pod (virt-launcher-*), vmi and vmirs objects in the eve-kube-app namespace.

The vmi lists cpu cores, memory Gi resources, and storage bytes requested.

The vmirs list cpu cores and memory Gi resources.

The virt-launcher pod lists two containers virt-launcher-monitor (the vm) and virt-tail (logs). Neither of these specifically list cores but instead a Millicore value which doesn't seem to match the core count. My virt-launcher reports 200millicores which is 1/5th of a core instead of the vmi and vmirs which report 2 cores. Using memory requests from the virt-launcher pod does seem to be the most complete view of all memory used by all containers associated with a running domain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zedi-pramodh I've changed KubeEVEAppPodMetrics to a generically named KubeEVEAppMetrics and KubeClusterMetrics now has two lists of it eve_pod_apps and eve_vmi_apps. How does this look?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zedi-pramodh I've changed KubeEVEAppPodMetrics to a generically named KubeEVEAppMetrics and KubeClusterMetrics now has two lists of it eve_pod_apps and eve_vmi_apps. How does this look?

That sounds right.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if that causes a change in controller API. Just cross check with @xyuria-zededa

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eriknordmark eriknordmark requested a review from deitch November 6, 2024 08:37
Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not holding it up for the naming question, so approved, but if you agree on it, would be good to change.

proto/metrics/metrics.proto Outdated Show resolved Hide resolved
For kubevirt-eve images this adds per node
and eve-user-app cpu/memory usage metrics.

Fix: add cluster_id to ZInfoKubeClusterUpdateStatus

Signed-off-by: Andrew Durbin <andrewd@zededa.com>
Signed-off-by: Andrew Durbin <andrewd@zededa.com>
@eriknordmark eriknordmark merged commit 8fc1248 into lf-edge:main Nov 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants