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

Finalize KubeVirt integrations #18744

Merged
merged 6 commits into from
Oct 4, 2024
Merged

Conversation

NouemanKHAL
Copy link
Member

@NouemanKHAL NouemanKHAL commented Oct 1, 2024

What does this PR do?

Handles some comments from the KubeVirt PRs:

Motivation

Synchronize all the 3 integrations to follow the same behavior:

  • Add missing metric units.
  • Handle namespace label collision with the kubename space (rename namespace -> vm_namespace).
  • Use the same metric description for the can_connect metric.

Additional Notes

For the kubevirt_handler, and kubevirt_controller metrics, the only metric that contains the namespace label are kubevirt_vmi and kubevirt_vm metrics, I confirmed that using:

➜ $ cat kubevirt_controller/tests/fixtures/metrics.txt | grep "namespace=" | cut -c1-12 | sort | uniq
kubevirt_vm_
kubevirt_vmi
➜  $ cat kubevirt_handler/tests/fixtures/metrics.txt | grep "namespace=" | cut -c1-12 | sort | uniq
kubevirt_vmi
➜  $ cat kubevirt_api/tests/fixtures/metrics.txt | grep "namespace=" | cut -c1-12 | sort | uniq
➜  

Both metrics share the same label values name="aws-kubevirt-vm-6",namespace="default" referring to the VM resource, example:

kubevirt_vm_starting_status_last_transition_timestamp_seconds{name="aws-kubevirt-vm-6",namespace="default"} 0
kubevirt_vmi_non_evictable{name="aws-kubevirt-vm-6",namespace="default",node="ip-192-168-58-28.eu-west-3.compute.internal"} 0

So we can safely rename the namespace label to vm_namespace to avoid collision with the namespace tag attached by the agent on Kubernetes which is different than the namespace where the VM is running.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Changelog entries must be created for modifications to shipped code
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.77%. Comparing base (fcbaaa1) to head (e073b32).
Report is 27 commits behind head on master.

Additional details and impacted files
Flag Coverage Δ
activemq ?
cassandra ?
hive ?
hivemq ?
hudi ?
ignite ?
jboss_wildfly ?
kafka ?
kubevirt_api 82.44% <ø> (ø)
kubevirt_controller 85.36% <ø> (ø)
kubevirt_handler 91.32% <ø> (ø)
presto ?
solr ?

Flags with carried forward coverage won't be shown. Click here to find out more.

@NouemanKHAL NouemanKHAL marked this pull request as ready for review October 4, 2024 09:52
@NouemanKHAL NouemanKHAL requested review from a team as code owners October 4, 2024 09:52
@NouemanKHAL NouemanKHAL changed the title Synchronize KubeVirt integrations Finalize KubeVirt integrations Oct 4, 2024
@NouemanKHAL NouemanKHAL merged commit 3150634 into master Oct 4, 2024
57 of 60 checks passed
@NouemanKHAL NouemanKHAL deleted the noueman/kubevirt-intgs-sync branch October 4, 2024 11:21
github-actions bot pushed a commit that referenced this pull request Oct 4, 2024
* rename namespace label to vmi_namespace for kubevirt_handler metrics

* images folders

* update the description of the can_connect metrics

* add process_signatures entries

* rename namespace label to vm_namespace for both vm and vmi metrics since they share the same name value

* add missing units to kubevirt_controller 3150634
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants