Collect VMI OS info from the Guest agent

[assafad]

What this PR does

Before this PR:
No guest agent info was collected and exposed through vmi metrics.
After this PR:
kernelRelease, machine, name and versionId fields from vmi.status.guestOsInfo are collected and added as labels to kubevirt_vmi_phase_count metric.

Fixes # https://issues.redhat.com/browse/CNV-37369

Why we need it and why it was done in this way

We would like to have more visibility regarding VMIs OS, so we can differentiate between running VMs, and alert for specific OS issues (e.g. https://issues.redhat.com/browse/CNV-38482).

Special notes for your reviewer

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

Collect VMI OS info from the Guest agent as `kubevirt_vmi_phase_count` metric labels

[sradco]

/lgtm

[assafad]

@enp0s3 can you please take a look?

[fabiand]

Hey.

Many of our metrics are limited to infra aspects.
This metric change captures my attention, because we start to disclose details about the guest.

This can be potentially security relevant information, thus:

1. What oter metrics do we have that are exporting guest related informations?
2. Did we consider to add a cluster level knob to disable all guest related exposure via metrics?

To me metrics stand out (compared to i.e. CR level details), because metrics are designed to be formwared to remote systems, and it's much easier to - unintentionally - leak guest related informations.

/hold

to give us time to answer these questions.

[assafad]

Hey.

Many of our metrics are limited to infra aspects. This metric change captures my attention, because we start to disclose details about the guest.

This can be potentially security relevant information, thus:

1. What oter metrics do we have that are exporting guest related informations?
2. Did we consider to add a cluster level knob to disable all guest related exposure via metrics?

To me metrics stand out (compared to i.e. CR level details), because metrics are designed to be formwared to remote systems, and it's much easier to - unintentionally - leak guest related informations.

/hold

to give us time to answer these questions.

@fabiand Hi, I'm not aware of any metrics exposing guest related information yet, thus we didn't consider implementing such knob. Are there specific GuestOSInfo fields exposed by this PR that you find problematic, or is it a general concern?

[fabiand]

It's a general concern - by exposing any information about the guest we cross this boundary.

Thus to me this PR should be extended with a cluster level knob in order to turn off all guest related metric reporting.

[fabiand]

/unhold

Usually metric system already require authentication, aka not permitting arbitrary users to view metrics.
Filtering can also be applied when metrics are forwarded to an out-of-cluster metrics collection instance.

This should be good enough.

[assafad]

/retest

[assafad]

/retest

[machadovilaca]

/lgtm

[enp0s3]

@assafad Can you please assert on non-empty strings on each one of the newly added fields?

[assafad]

Added.

[enp0s3]

/approve

[kubevirt-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enp0s3

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:

• pkg/monitoring/OWNERS [enp0s3]
• tests/monitoring/OWNERS [enp0s3]

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]

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

[assafad]

/retest

[assafad]

/cherry-pick release-1.2

[kubevirt-bot]

@assafad: new pull request created: #11717

In response to this:

/cherry-pick release-1.2

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/test-infra repository.