Skip to content

Conversation

@josemacassan
Copy link
Contributor

@josemacassan josemacassan commented Nov 19, 2025

Short description:

Backport: #2665

Summary by CodeRabbit

Release Notes

  • New Features

    • Added deterministic randomized IP address generation across network tests.
    • Added unique DHCP client identifier support for network configurations.
    • Enhanced storage checkup timeout error reporting with pod diagnostics.
  • Bug Fixes

    • Replaced Jira-based test gating with xfail markers for improved stability tracking.
    • Improved console connection retry behavior with explicit timeout handling.
  • Updates

    • Updated default Fedora image version from 41 to 43.
    • Updated operator image tag to cnv-4.20.
    • Simplified test infrastructure by removing network addons operator tests and Renovate configuration.
  • Refactoring

    • Refactored Linux user information retrieval to use VM objects.
    • Streamlined data volume creation with new helper functions.

josemacassan and others added 2 commits November 19, 2025 14:33
…h_unprivileged_client_no_permissions (RedHatQE#2665)

##### Short description:
Improving test
`test_fail_to_vmexport_with_unprivileged_client_no_permissions` to use a
blank data source with unprivileged lcient to create a vmexport
instance.
##### More details:

##### What this PR does / why we need it:
Change the test to use a blank data source instead a cirros one
##### Which issue(s) this PR fixes:

##### Special notes for reviewer:

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


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Improved VM export tests: added an admin-created data-volume fixture,
renamed the unprivileged-created fixture, removed redundant readiness
waits, and simplified parameterization to rely on the new fixtures.
* **Utilities**
* Added a reusable helper to create blank data volumes for tests,
consolidating creation and readiness handling to reduce setup
duplication.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@coderabbitai
Copy link

coderabbitai bot commented Nov 19, 2025

Caution

Review failed

Failed to post review comments

Walkthrough

This pull request updates the project to target CNV version 4.20, upgrades Fedora image support from 41/42 to 43, replaces hard-coded test IP addresses with deterministic random generation, removes unstable CNAO network tests, and refactors test fixtures and utilities for improved maintainability and reliability.

Changes

Cohort / File(s) Summary
Version and Build Configuration
Makefile, pyproject.toml, tox.ini
Updates default operator image tag to cnv-4.20, changes openshift-python-wrapper dependency to ~=4.20.0, and updates polarion/Jira verification targets to 4.20 branch.
Fedora Image Upgrades
libs/infra/images.py, utilities/constants.py, utilities/os_utils.py, tests/global_config_x86_64.py
Removes FEDORA41_IMG, adds FEDORA43_IMG to image dataclass and constants, updates LATEST_RELEASE_STR pointer and FEDORA_OS_MAPPING.
Dependency Automation Removal
renovate.json
Deletes entire Renovate configuration file, removing automated dependency update scheduling and rules.
Golden Image Fetching
tests/infrastructure/golden_images/conftest.py
Replaces local filesystem parsing with HTTP request to Fedora releases JSON API for latest_fedora_release_version, removes downloaded_latest_libosinfo_db parameter.
Network Test Infrastructure
tests/network/libs/ip.py, tests/network/libs/dhcpd.py
Increases random octet pool from 4 to 16, introduces UNIQUE_CLIENT_ID constant for DHCP, replaces hard-coded subnet/IP ranges with random_ipv4_address-derived values.
Network IP Address Randomization
tests/network/bgp/conftest.py, tests/network/l2_bridge/conftest.py, tests/network/l2_bridge/test_ovs_bridge.py, tests/network/macspoof/conftest.py, tests/network/migration/test_migration.py, tests/network/upgrade/conftest.py, tests/network/utils.py
Replaces hard-coded IPv4 addresses with deterministic random generation via random_ipv4_address utility.
Network Test Removals and Simplifications
tests/network/cluster_addons_operator/test_network_addons_operator.py, tests/network/connectivity/test_ovs_linux_bridge.py, tests/network/l2_bridge/test_l2_ovs_linux_bridge.py, tests/network/user_defined_network/test_user_defined_network.py
Removes entire CNAO test module, removes single_nic marker from test_ipv6_linux_bridge, removes OVS bridge quarantine from DHCP test, removes QUARANTINED import and xfail from connectivity test.
Network Test Fixture Updates
tests/network/localnet/conftest.py, tests/network/secondary_network_dns/test_secondary_network_dns.py, tests/network/network_policy/test_network_policy.py
Updates migrated_localnet_vm to accept localnet_vms_have_connectivity parameter, updates nslookup command with timeout/retry options and random IP, refactors ApplyNetworkPolicy constructor.
Golden Images Test Updates
tests/infrastructure/golden_images/update_boot_source/test_ssp_common_templates_boot_sources.py
Imports OS_FLAVOR_FEDORA constant and replaces substring check with constant comparison.
Storage Test Refactoring
tests/storage/cdi_import/test_import_http.py, tests/storage/hpp/test_hostpath.py, tests/storage/checkups/conftest.py, tests/storage/conftest.py, tests/storage/vm_export/conftest.py, tests/storage/vm_export/test_vm_export.py, tests/storage/vm_export/utils.py
Removes obsolete storage test helpers and fixtures, adds unprivileged_client parameter to Job creation with enhanced timeout error handling, introduces blank_dv_created_by_* fixtures and create_blank_dv_by_specific_user helper, removes Jira-based gating and adds container-based xfail.
Storage Test Quarantines
tests/storage/cdi_import/test_import_http.py, tests/storage/test_libguestfs.py, tests/storage/restricted_namespace_cloning/test_restricted_namespace_cloning.py, tests/install_upgrade_operators/csv/test_csv_permissions_audit.py
Adds QUARANTINED import and xfail decorators to multiple test functions, removes gating marker from restricted namespace test.
Utility Enhancements
utilities/console.py, utilities/exceptions.py
Adds Console.init type annotations and TIMEOUT_30SEC default, decorates connect with @retry, adds StorageCheckupConditionTimeoutExpiredError exception.
Common Templates Utils
tests/virt/cluster/common_templates/utils.py
Updates get_linux_user_info signature from ssh_exec to vm parameter with OS-version-specific parsing logic.
Test Conftest Updates
tests/conftest.py
Removes InstallPlan import and is_post_cnv_upgrade_cluster fixture.
Unit Test Updates
utilities/unittests/conftest.py, utilities/unittests/test_console.py, utilities/unittests/test_os_utils.py
Updates Fedora image constants from 41 to 43, removes timeout specifications from console expect calls, updates OS utils tests for fedora-43.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

  • tests/storage/cdi_import/test_import_http.py — Large refactoring with multiple test signature changes, parametrization restructuring, and xfail marker additions; verify all test intents remain intact.
  • tests/network/l2_bridge/conftest.py and related L2 bridge tests — Extensive replacement of hard-coded IPs with random generation; verify the seeding strategy maintains deterministic reproducibility.
  • tests/storage/checkups/conftest.py — New exception handling flow with pod log retrieval; ensure error cases are properly handled and diagnostics are correct.
  • tests/virt/cluster/common_templates/utils.py — Function signature and logic changes for get_linux_user_info; verify all call sites are updated and OS-specific parsing logic is correct.
  • utilities/console.py — New type annotations, retry decorator, and removal of explicit timeouts; verify console connection behavior under various timeout scenarios.
  • Fedora version migration (libs/infra/images.py, utilities/constants.py, utilities/os_utils.py, tests/global_config_x86_64.py) — Ensure all references to FEDORA41_IMG are removed and FEDORA43_IMG is properly integrated across image handling.

Possibly related PRs

Suggested labels

verified, size/M, can-be-merged, branch-main, sig-storage, cherry-pick-cnv-4.20

Suggested reviewers

  • jpeimer
  • duyanyan
  • kgoldbla
  • dalia-frank
  • stesrn

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning Description is incomplete; it only provides a backport reference without covering the required template sections (What/Why, Issue, Special notes, Jira ticket). Complete the PR description by filling in template sections: More details, What/Why, Issue fixes, Special notes, and Jira ticket URL or 'NONE'.
Docstring Coverage ⚠️ Warning Docstring coverage is 18.84% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed Title directly addresses the main change: replacing Cirros with blank DV in a specific test, which is a central modification visible in the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Nov 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (cnv-4.20@5807c6e). Learn more about missing BASE report.

Additional details and impacted files
@@             Coverage Diff             @@
##             cnv-4.20    #2742   +/-   ##
===========================================
  Coverage            ?   93.51%           
===========================================
  Files               ?       16           
  Lines               ?     1358           
  Branches            ?        0           
===========================================
  Hits                ?     1270           
  Misses              ?       88           
  Partials            ?        0           
Flag Coverage Δ
utilities 93.51% <ø> (?)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@openshift-virtualization-qe-bot

D/S test tox -e verify-tc-requirement-polarion failed: cnv-tests-tox-executor/19834

Copy link

Choose a reason for hiding this comment

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

D/S test tox -e verify-bugs-are-open failed: cnv-tests-tox-executor/19837

@josemacassan
Copy link
Contributor Author

/retest tox

Copy link

Choose a reason for hiding this comment

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

D/S test tox -e verify-bugs-are-open failed: cnv-tests-tox-executor/19845

@jpeimer
Copy link
Contributor

jpeimer commented Nov 19, 2025

/approve
/lgtm

@Ahmad-Hafe
Copy link
Contributor

/lgtm

@rnetser
Copy link
Collaborator

rnetser commented Nov 20, 2025

/approve
/lgtm

@josemacassan
Copy link
Contributor Author

/verified

@openshift-virtualization-qe-bot-3
Copy link
Contributor

@coderabbitai

Test execution plan request details

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will create a change request comment on the 1st line of the 1st file in the pr with the test execution plan.
If you fail to run or post a comment, retry.

Analysis Requirements:

  1. Examine code changes in each modified file
  2. Identify affected code paths, functions, and classes
  3. Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes
  4. Trace test dependencies through imports, shared utilities, and fixture inheritance
  5. Detect new tests introduced in the PR

Your deliverable:
Your change request comment will be based on the following requirements:

Test Execution Plan

  • path/to/test_file.py - When the entire test file needs verification
  • path/to/test_file.py::TestClass::test_method - When specific test(s) needed
  • path/to/test_file.py::test_function - When specific test(s) needed
  • -m marker - When specific marker(s) can be used to cover multiple cases.

Guidelines:

  • Include only tests directly affected by the changes
  • Use a full file path only if ALL tests in that file require verification
  • Use file path + test name if only specific tests are needed
  • If a test marker can cover multiple files/tests, provide the marker
  • Balance coverage vs over-testing - Keep descriptions minimal
  • Do not add a follow-up comment in the PR, only the change request one

@coderabbitai
Copy link

coderabbitai bot commented Nov 21, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the list of modified files in the PR to identify the first file
cd /tmp && gh pr view 2742 --repo RedHatQE/openshift-virtualization-tests --json files --jq '.files[].path' | head -20

Length of output: 110


🏁 Script executed:

#!/bin/bash
# Examine the main vm_export test file to see the specific changes
cat tests/storage/vm_export/test_vm_export.py | head -100

Length of output: 2817


🏁 Script executed:

#!/bin/bash
# Check what tests exist in vm_export
ast-grep --pattern 'def test_$_($$):
  $$$'  tests/storage/vm_export/test_vm_export.py

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Examine the conftest changes for vm_export fixtures
cat tests/storage/vm_export/conftest.py

Length of output: 8027


🏁 Script executed:

#!/bin/bash
# Check network tests that were modified
rg -l "random_ipv4_address" tests/network/ --type py

Length of output: 657


🏁 Script executed:

#!/bin/bash
# Get all files changed in PR 2742
gh pr view 2742 --repo RedHatQE/openshift-virtualization-tests --json files --jq '.files[].path'

Length of output: 110


🏁 Script executed:

#!/bin/bash
# Get the PR diff to see actual changes
gh pr diff 2742 --repo RedHatQE/openshift-virtualization-tests | head -200

Length of output: 6968


🏁 Script executed:

#!/bin/bash
# Check the tests in vm_export directory
rg "^def test_" tests/storage/vm_export/test_vm_export.py

Length of output: 144


@openshift-virtualization-qe-bot-3,

Based on my analysis of PR #2742, this PR makes focused changes to the VM export tests, specifically replacing the use of a Cirros image DataVolume with a blank DataVolume in tests that verify unprivileged client behavior.

Test Execution Plan

The following tests are directly affected by the fixture and implementation changes:

  • tests/storage/vm_export/test_vm_export.py::test_fail_to_vmexport_with_unprivileged_client_no_permissions - Modified to use blank DV created by admin user instead of Cirros image
  • tests/storage/vm_export/test_vm_export.py::test_virtctl_vmexport_unprivileged - Updated to use renamed fixture for unprivileged user DV

These two tests should be executed to verify:

  1. The new blank_dv_created_by_admin_user fixture works correctly
  2. The renamed blank_dv_created_by_unprivileged_user fixture functions as expected
  3. Both tests maintain their original functionality with blank DVs instead of image-based DVs

🧠 Learnings used
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.

Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds architecture-specific OS_FLAVOR attributes to the Images.Cirros class (OS_FLAVOR_CIRROS for x86_64/ARM64, OS_FLAVOR_FEDORA for s390x), enabling conditional logic based on the underlying OS flavor in tests.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: The test execution plan for PR #1904 focuses on cluster-type conditional logic where nmstate functionality is bypassed on cloud clusters (Azure/AWS) but fully functional on bare-metal/PSI clusters, requiring different test strategies for each environment type.

Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds the OS_FLAVOR attribute to the Images.Cirros class, making Images.Cirros.OS_FLAVOR available for conditional logic in tests.

Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.

Learnt from: chandramerla
Repo: RedHatQE/openshift-virtualization-tests PR: 2577
File: tests/virt/node/hotplug/test_cpu_memory_hotplug.py:161-162
Timestamp: 2025-11-19T17:00:58.229Z
Learning: In the openshift-virtualization-tests repository, the s390x test execution strategy uses positive filtering: only tests explicitly marked with pytest.mark.s390x are executed on s390x clusters. Tests without the s390x marker are automatically excluded from s390x runs, so explicit skipif decorators are not needed to prevent execution on s390x.

Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.

Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:83-97
Timestamp: 2025-06-23T19:19:31.961Z
Learning: In OpenShift Virtualization mass machine type transition tests, the kubevirt_api_lifecycle_automation_job requires cluster-admin privileges to function properly, as confirmed by the test maintainer akri3i.

Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: libs/net/node_network.py:25-31
Timestamp: 2025-08-20T23:43:28.117Z
Learning: In the RedHatQE/openshift-virtualization-tests project, servolkov's team always uses bare metal (BM) clusters with IPv4 setup in their testing environment, making defensive checks for IPv4 data presence potentially redundant in their networking code.

Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.

Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:24-64
Timestamp: 2025-06-23T19:28:20.281Z
Learning: In OpenShift Virtualization mass machine type transition tests, the machine type glob pattern "pc-q35-rhel8.*.*" is intentionally hard-coded in the kubevirt_api_lifecycle_automation_job as it's used only once for this specific test case, with plans to update it in the future if the job needs to support other machine types.

Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1411
File: utilities/os_utils.py:246-279
Timestamp: 2025-07-22T17:13:59.166Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, CentOS preferences follow the format "centos-stream<version>" (e.g., "centos-stream9", "centos-stream10"). The generate_instance_type_centos_os_matrix function correctly uses regex to extract numeric versions and constructs the latest version string in the same format as the input preferences for proper comparison.

Learnt from: SiboWang1997
Repo: RedHatQE/openshift-virtualization-tests PR: 1566
File: tests/global_config_x86_64.py:29-29
Timestamp: 2025-07-25T01:59:02.180Z
Learning: In the openshift-virtualization-tests repo, the s390x architecture still relies on Fedora 41 images, while only x86_64 is updated to Fedora 42 in PR #1566.

Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2014
File: tests/storage/cdi_clone/test_clone.py:0-0
Timestamp: 2025-09-10T13:25:59.799Z
Learning: In the openshift-virtualization-tests repository, when creating DataVolume objects using ocp_resources, use `api_name="storage"` rather than the default `api_name="pvc"`.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1723
File: conftest.py:496-502
Timestamp: 2025-08-27T12:38:26.496Z
Learning: In PR #1723, the user rnetser clarified that updating downstream string comparisons to handle DataVolume enums from StorageClassConfig is not part of the current PR scope, which focuses only on adding conformance storage class support.

Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 2249
File: tests/install_upgrade_operators/must_gather/test_must_gather.py:428-441
Timestamp: 2025-10-16T12:47:04.521Z
Learning: In openshift-virtualization-tests repository, DataVolumes in the openshift-virtualization-os-images namespace are volatile resources managed by DataImportCron. They can be created/destroyed between must-gather collection listing and file retrieval, requiring FileNotFoundError exception handling in test_crd_resources to skip these volatile resources gracefully while still validating DataVolumes in other namespaces. There is no pytest_generate_tests hook that filters out datavolumes from test parametrization.

@kshvaika
Copy link
Contributor

/lgtm

@josemacassan
Copy link
Contributor Author

josemacassan commented Nov 24, 2025

/verified

By running locally pytest tests/storage/vm_export/test_vm_export.py

Cluster info:

Name: stg-jose420
Domain: on-rhos-ipi
Size: Standard
CNV Version: 4.20.0

@rnetser rnetser merged commit 5993297 into RedHatQE:cnv-4.20 Nov 24, 2025
34 checks passed
@openshift-virtualization-qe-bot-2

New container for quay.io/openshift-cnv/openshift-virtualization-tests:cnv-4.20 published

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.

8 participants