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

[release-v1.59] Progress metrics refactor and rename #3270

Conversation

arnongilboa
Copy link
Collaborator

@arnongilboa arnongilboa commented May 18, 2024

What this PR does / why we need it:

Manual backport of #3254, #3292, #3293

clone_progress, openstack_populator_progress and ovirt_progress metrics are not in the monitoring package. These metrics are not with correct metrics names, based on the kubevirt and Prometheus metrics naming conventions, they are not documented and are not located under /pkg/monitoring. After the code refactoring we should not have Prometheus metrics in other places in the code, other than the /monitoring/metrics package, and metrics should be registered using operator-observability package.

Align the progress metric names with the metric naming linter rules: kubevirt_cdi_clone_progress_total, kubevirt_cdi_openstack_populator_progress_total, kubevirt_cdi_ovirt_progress_total. Add kubevirt_cdi_import_progress_total metric and use it in the importer instead of kubevirt_cdi_clone_progress_total.

Fix the metric parsing to match their new names. Otherwise the DV progress will not be updated until its 100%.

Fix progress reporting for http imports.

Which issue(s) this PR fixes:
Fixes #https://issues.redhat.com/browse/CNV-36845

Special notes for your reviewer:

Release note:

Align progress metrics names with kubevirt and Prometheus metrics naming conventions

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels May 18, 2024
@kubevirt-bot kubevirt-bot requested review from aglitke and alromeros May 18, 2024 19:19
Manual backport of kubevirt#3254

clone_progress, openstack_populator_progress and ovirt_progress metrics
are not in the monitoring package. These metrics are not with correct
metrics names, based on the kubevirt and Prometheus metrics naming
conventions, they are not documented and are not located under
/pkg/monitoring. After the code refactoring we should not have
Prometheus metrics in other places in the code, other than the
/monitoring/metrics package, and metrics should be registered using
operator-observability package.

In addition, we aligned the progress metric names with the metric
naming linter rules:
kubevirt_cdi_clone_progress_total
kubevirt_cdi_openstack_populator_progress_total
kubevirt_cdi_ovirt_progress_total

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
@arnongilboa arnongilboa force-pushed the clone_progress_metric_refactor-v1.59 branch from bfe42fc to 1304ee8 Compare May 18, 2024 19:25
@akalenyu
Copy link
Collaborator

/test pull-containerized-data-importer-e2e-destructive-release
/hold
cancel if we must have this soon

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 19, 2024
arnongilboa and others added 2 commits June 5, 2024 12:53
Manual backport of kubevirt#3292

Use default metric registration. We shouldn't use the controller-runtime
registration as we have no controller here and it will not register the
metric correctly.

Add kubevirt_cdi_import_progress_total metric and use it in the importer
instead of kubevirt_cdi_clone_progress_total.

Fix the metric parsing to match their new names. Otherwise the DV
progress will not be updated until its 100%.

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
Backport of kubevirt#3293

* tests: fix regex in import progress test

This test currently passes since the regex will always catch 100%,
hiding a bug we have in progress reporting.

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* Fix actual progress reporting for url->scratch

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* Hold on with reporting 100% until pod exits

Otherwise, we'll be reporting 100% prior to convert/resize/... being
done

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* tests: fix a test that was adapted to no progress reporting

this tests cares about some progress being made so N/A
isn't the right choice

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

---------

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
@akalenyu
Copy link
Collaborator

/approve
/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 18, 2024
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akalenyu

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-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 18, 2024
@akalenyu
Copy link
Collaborator

/hold cancel

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 10, 2024
@kubevirt-bot kubevirt-bot merged commit edb62b5 into kubevirt:release-v1.59 Jul 10, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants