Add kubevirt_vm/i_vnic_info metrics #13610

avlitman · 2024-12-29T11:00:31Z

What this PR does

Add 2 metrics that reports the vNIC information for vm's and vmi's.
the metrics labels includes:
- name: the vm/i name
- namespace: the vm/i namespace
- vnic_name: the interface name
- binding_type: can be core/plugin
- network: if using Pod it will be "pod networking", if using Multus it will be the Multus.NetworkName.
- binding_name: can be masquerade/bridge/sriov/Binding.Name

see metrics:

notes:

The vmi metric will show values for running/pending/starting vmi's but not for stopped vmi's.
The vm metric will show values for both running/starting/stopped vms, but only for vms that the network and interface defined on their spec.

Fixes #
jira-ticket: https://issues.redhat.com/browse/CNV-51529

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Design: A design document was considered and is present (link) or not required
PR: The PR description is expressive enough and will help future contributors
Code: Write code that humans can understand and Keep it simple
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
Upgrade: Impact of this change on upgrade flows was considered and addressed if required
Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
Community: Announcement to kubevirt-dev was considered

Release note

Added kubevirt_vm_vnic_info and kubevirt_vmi_vnic_info metrics

kubevirt-bot · 2024-12-29T11:00:34Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

pkg/monitoring/metrics/virt-controller/vmstats_collector.go

avlitman · 2024-12-31T13:02:34Z

@EdDev @orelmisan Will appreciate your review

nunnatsa

The CollectVmsVnicInfo and the CollectVmisVnicInfo functions are very similar and contains a lot of duplicate code. I'm sure we can take parts of this functions into common functions.

For example, this code:

switch {
		case iface.Masquerade != nil:
			bindingType = "masquerade"
		case iface.Bridge != nil:
			bindingType = "bridge"
		case iface.SRIOV != nil:
			bindingType = "sriov"
		case iface.Binding != nil:
			bindingType = "binding"
			bindingValue = iface.Binding.Name
		}

Can be changed into something like:

func getBinding(iface) (string,string {
 	bindingType := none
	bindingValue := none

	switch {
	case iface.Masquerade != nil:
		bindingType = "masquerade"
	case iface.Bridge != nil:
		bindingType = "bridge"
	case iface.SRIOV != nil:
		bindingType = "sriov"
	case iface.Binding != nil:
		bindingType = "binding"
		bindingValue = iface.Binding.Name
	}
	return bindingType, bindingValue
}

...
bindingType, bindingValue := getBinding(iface)

docs/observability/metrics.md

pkg/monitoring/metrics/virt-controller/vmistats_collector.go

pkg/monitoring/metrics/virt-controller/vmstats_collector.go

sradco · 2025-01-01T12:06:37Z

@avlitman
Do we have cases where vm.Spec.Template.Spec.Domain.Devices.Interfaces or vm.Spec.Template.Spec.Networks is empty?
Do we need to handle them in the code?

pkg/monitoring/metrics/virt-controller/vmistats_collector_test.go

pkg/monitoring/metrics/virt-controller/vmistats_collector.go

pkg/monitoring/metrics/virt-controller/vmstats_collector.go

avlitman · 2025-01-01T12:11:50Z

Do we have cases where vm.Spec.Template.Spec.Domain.Devices.Interfaces or vm.Spec.Template.Spec.Networks is empty?
Do we need to handle them in the code?

Yes we have cases like this for vms but not for vmi, vmi should contain the interface and network information all the time. and the vm shows this info only when both network and interface applied in the vm yaml (you have to define both- if not it fail to create the vm)

EdDev · 2025-02-04T14:31:18Z

Do we have cases where vm.Spec.Template.Spec.Domain.Devices.Interfaces or vm.Spec.Template.Spec.Networks is empty?
Do we need to handle them in the code?

Yes we have cases like this for vms but not for vmi, vmi should contain the interface and network information all the time. and the vm shows this info only when both network and interface applied in the vm yaml (you have to define both- if not it fail to create the vm)

It is a valid configuration to have a VM with no network interfaces.
Through the UI, you can make this happen easily. Directly using manifests, there is a need to add some knobs to assure no defaults are used.

pkg/monitoring/metrics/virt-controller/vmstats_collector.go

Signed-off-by: avlitman <[email protected]>

kubevirt-bot · 2025-02-05T09:29:16Z

@avlitman: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test pull-kubevirt-apidocs
/test pull-kubevirt-build
/test pull-kubevirt-build-arm64
/test pull-kubevirt-build-s390x
/test pull-kubevirt-check-unassigned-tests
/test pull-kubevirt-client-python
/test pull-kubevirt-code-lint
/test pull-kubevirt-e2e-k8s-1.30-sig-compute
/test pull-kubevirt-e2e-k8s-1.30-sig-network
/test pull-kubevirt-e2e-k8s-1.30-sig-operator
/test pull-kubevirt-e2e-k8s-1.30-sig-storage
/test pull-kubevirt-e2e-k8s-1.31-sig-compute
/test pull-kubevirt-e2e-k8s-1.31-sig-compute-migrations
/test pull-kubevirt-e2e-k8s-1.31-sig-monitoring
/test pull-kubevirt-e2e-k8s-1.31-sig-network
/test pull-kubevirt-e2e-k8s-1.31-sig-operator
/test pull-kubevirt-e2e-k8s-1.31-sig-performance
/test pull-kubevirt-e2e-k8s-1.31-sig-storage
/test pull-kubevirt-e2e-k8s-1.32-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.32-sig-compute
/test pull-kubevirt-e2e-k8s-1.32-sig-compute-serial
/test pull-kubevirt-e2e-k8s-1.32-sig-network
/test pull-kubevirt-e2e-k8s-1.32-sig-operator
/test pull-kubevirt-e2e-k8s-1.32-sig-storage
/test pull-kubevirt-e2e-kind-1.30-vgpu
/test pull-kubevirt-e2e-kind-sriov
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-fossa
/test pull-kubevirt-generate
/test pull-kubevirt-manifests
/test pull-kubevirt-prom-rules-verify
/test pull-kubevirt-unit-test
/test pull-kubevirt-unit-test-s390x
/test pull-kubevirt-verify-go-mod

The following commands are available to trigger optional jobs:

/test build-kubevirt-builder
/test pull-kubevirt-check-tests-for-flakes
/test pull-kubevirt-conformance-arm64
/test pull-kubevirt-e2e-arm64
/test pull-kubevirt-e2e-k8s-1.31-sig-compute-conformance
/test pull-kubevirt-e2e-k8s-1.31-sig-compute-root
/test pull-kubevirt-e2e-k8s-1.31-sig-storage-root
/test pull-kubevirt-e2e-k8s-1.32-sig-compute-realtime
/test pull-kubevirt-e2e-k8s-1.32-sig-network-multus-v4
/test pull-kubevirt-e2e-k8s-1.32-sig-performance-kwok
/test pull-kubevirt-e2e-k8s-1.32-single-node
/test pull-kubevirt-e2e-k8s-1.32-swap-enabled
/test pull-kubevirt-fuzz
/test pull-kubevirt-gosec
/test pull-kubevirt-goveralls
/test pull-kubevirt-metrics-lint
/test pull-kubevirt-unit-test-arm64
/test pull-kubevirt-verify-rpms

Use /test all to run the following jobs that were automatically triggered:

pull-kubevirt-apidocs
pull-kubevirt-build
pull-kubevirt-build-arm64
pull-kubevirt-build-s390x
pull-kubevirt-check-tests-for-flakes
pull-kubevirt-check-unassigned-tests
pull-kubevirt-client-python
pull-kubevirt-code-lint
pull-kubevirt-conformance-arm64
pull-kubevirt-e2e-arm64
pull-kubevirt-e2e-k8s-1.31-sig-compute-migrations
pull-kubevirt-e2e-k8s-1.31-sig-monitoring
pull-kubevirt-e2e-k8s-1.31-sig-performance
pull-kubevirt-e2e-k8s-1.32-sig-compute
pull-kubevirt-e2e-k8s-1.32-sig-compute-serial
pull-kubevirt-e2e-k8s-1.32-sig-network
pull-kubevirt-e2e-k8s-1.32-sig-operator
pull-kubevirt-e2e-k8s-1.32-sig-storage
pull-kubevirt-fossa
pull-kubevirt-fuzz
pull-kubevirt-generate
pull-kubevirt-goveralls
pull-kubevirt-manifests
pull-kubevirt-prom-rules-verify
pull-kubevirt-unit-test
pull-kubevirt-unit-test-arm64
pull-kubevirt-unit-test-s390x
pull-kubevirt-verify-go-mod

In response to this:

/retest pull-kubevirt-check-tests-for-flakes

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 kubernetes-sigs/prow repository.

avlitman · 2025-02-05T09:30:31Z

/test pull-kubevirt-check-tests-for-flakes

EdDev

Thanks!

sradco · 2025-02-06T10:18:26Z

/approve

kubevirt-bot · 2025-02-06T10:19:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sradco

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kubevirt-commenter-bot · 2025-02-06T10:19:15Z

Required labels detected, running phase 2 presubmits:
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-e2e-kind-1.30-vgpu
/test pull-kubevirt-e2e-kind-sriov
/test pull-kubevirt-e2e-k8s-1.32-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.30-sig-network
/test pull-kubevirt-e2e-k8s-1.30-sig-storage
/test pull-kubevirt-e2e-k8s-1.30-sig-compute
/test pull-kubevirt-e2e-k8s-1.30-sig-operator
/test pull-kubevirt-e2e-k8s-1.31-sig-network
/test pull-kubevirt-e2e-k8s-1.31-sig-storage
/test pull-kubevirt-e2e-k8s-1.31-sig-compute
/test pull-kubevirt-e2e-k8s-1.31-sig-operator

kubevirt-commenter-bot · 2025-02-07T04:07:14Z

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

avlitman changed the title ~~Add kubevirt_vm_vnic_info metric~~ Add kubevirt_vm/i_vnic_info metrics Dec 29, 2024

kubevirt-bot requested review from machadovilaca and vladikr December 29, 2024 11:00

avlitman force-pushed the add-vnic-metric branch 2 times, most recently from 72bb514 to 95efe1f Compare December 29, 2024 13:11

avlitman marked this pull request as ready for review December 29, 2024 13:16

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 29, 2024

kubevirt-bot requested a review from lyarwood December 29, 2024 13:16

dosubot bot added area/monitoring kind/enhancement sig/network labels Dec 29, 2024

avlitman force-pushed the add-vnic-metric branch 2 times, most recently from 1aea25e to 3e5df14 Compare December 29, 2024 14:44

avlitman commented Dec 29, 2024

View reviewed changes

pkg/monitoring/metrics/virt-controller/vmstats_collector.go Outdated Show resolved Hide resolved

avlitman force-pushed the add-vnic-metric branch 2 times, most recently from 953e71f to eded0a6 Compare December 31, 2024 21:17

nunnatsa reviewed Jan 1, 2025

View reviewed changes