Skip to content

[4.22] Storage: Change Cirros images for RHEL from DataSource into upgrade tests#5226

Open
josemacassan wants to merge 1 commit into
RedHatQE:cnv-4.22from
josemacassan:improve-datasource-upgrade-test-4.20-cnv-4.22
Open

[4.22] Storage: Change Cirros images for RHEL from DataSource into upgrade tests#5226
josemacassan wants to merge 1 commit into
RedHatQE:cnv-4.22from
josemacassan:improve-datasource-upgrade-test-4.20-cnv-4.22

Conversation

@josemacassan

@josemacassan josemacassan commented Jun 15, 2026

Copy link
Copy Markdown
Contributor
What this PR does / why we need it:

Forwardport: #2917

Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:

Summary by CodeRabbit

Release Notes

  • Chores

    • Updated openshift-python-wrapper dependency to version 11.0.131.
  • Tests

    • Enhanced test infrastructure with improved fixtures and timeout handling for network and storage testing scenarios.
    • Migrated storage upgrade tests from Cirros to RHEL virtual machines.
    • Updated Jira issue tracking references across test suites.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (5)
  • main
  • cnv-4.21
  • cnv-4.20
  • cnv-4.19
  • cnv-4.18

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6680c5b2-d483-49ad-82f9-c5bde781907c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR performs CNV-4.22 branch maintenance: rotates stale Jira tracking IDs to CNV-88xxx tickets, activates all l2_bridge migration stuntime tests with affinity controls, implements the localnet NAD VLAN-change test, migrates upgrade storage tests from Cirros to RHEL VMs, adds a predictable-name snapshot restore test, extracts update_nad_references to a shared library, and hardens SSH commands with explicit 2-minute timeouts.

Changes

Jira ID rotation and Tox branch targeting for CNV-4.22

Layer / File(s) Summary
Jira fixture additions, removals, and ID changes
tests/install_upgrade_operators/conftest.py, tests/install_upgrade_operators/deployment/conftest.py, tests/install_upgrade_operators/deployment/test_hco_deployment_params.py, tests/install_upgrade_operators/pod_validation/test_pod_spec.py
Adds jira_88737_open, updates jira_86102_open to CNV-88764, removes jira_86639_open and its DISABLE_MDEV_CONFIGURATION branch, renames xfail migration-controller helpers to Jira-88737, and propagates new fixture names into test signatures.
Jira marker ID replacements and removed Jira gates across tests
tests/network/bgp/evpn/test_evpn_connectivity.py, tests/network/l2_bridge/bandwidth/test_bandwidth.py, tests/network/l2_bridge/migration_stuntime/test_migration_stuntime.py, tests/network/localnet/test_default_bridge.py, tests/network/network_service/conftest.py, tests/observability/conftest.py, tests/observability/metrics/conftest.py, tests/observability/metrics/test_vms_metrics.py, tests/observability/runbook_url/conftest.py, tests/storage/cdi_upload/test_upload_virtctl.py, tests/storage/test_hotplug.py, tests/virt/cluster/common_templates/fedora/test_fedora_os_support.py, tests/virt/node/general/test_cloud_init_disk_vm.py, tests/virt/node/hyperv_support/test_windows_crash_detection.py
Replaces stale @pytest.mark.jira IDs (CNV-76657→88741, CNV-84023→88738, CNV-86961 removed, CNV-87521→88742, CNV-88765 runbook), removes Jira-gated svc.clean_up() and hotplug resource-request logic, unxfails localnet default bridge test, adds skip-jira-utils-check annotations.
Tox branch and target-version updates
tox.ini
Targets upstream/cnv-4.22 for Polarion TC verification and removes 5.0.0 from --target-versions in the bug verification env.

Network test expansion: stuntime, localnet NAD ref-change, shared lib refactor

Layer / File(s) Summary
NAD update helper extracted to shared library
tests/network/libs/nad_ref.py, tests/network/l2_bridge/nad_ref_change/lib_helpers.py, tests/network/l2_bridge/nad_ref_change/test_nad_ref_change.py
Removes update_nad_references from the l2_bridge-local helper, adds the canonical implementation to tests/network/libs/nad_ref.py with MigrationRequired condition synchronization, and updates l2_bridge test imports to use the new location.
Stuntime library tuning and l2_bridge stuntime test activation
tests/network/libs/stuntime.py, tests/network/l2_bridge/migration_stuntime/conftest.py, tests/network/l2_bridge/migration_stuntime/test_migration_stuntime.py, tests/network/localnet/migration_stuntime/test_migration_stuntime.py
Changes PING_INTERVAL_SECONDS from 0.1 to 0.01, updates log format to two decimal places, adds CLIENT_VM_LABEL label to client VM, refactors NAD CNIPluginBridgeConfig, activates all five migration test methods (removes __test__ = False), wires l2_bridge_ip_family, and adds affinity/anti-affinity + stuntime assertion to each test body.
Localnet library constants and runcmd parameter
tests/network/localnet/liblocalnet.py
Adds IFACE_A_NAME, IFACE_B_NAME, CUDN_B_NAME constants and extends localnet_vm with an optional runcmd parameter wired into cloud-init user-data.
Localnet NAD ref-change test fixtures and test implementation
tests/network/localnet/nad_ref_change/conftest.py, tests/network/localnet/nad_ref_change/test_nad_ref_change.py
Adds full conftest with CUDN, reference VM, under-test VM, and baseline connectivity fixtures; implements test_running_vm_vlan_change to switch VLAN-A→VLAN-B, verify new connectivity per IP via subtests, and assert VLAN-A connectivity is absent.

Storage test migration from Cirros to RHEL and new snapshot restore test

Layer / File(s) Summary
Storage utility and data_volume_template cleanup
utilities/storage.py, tests/storage/conftest.py
Removes run_command_on_cirros_vm_and_check_output, changes data_volume_template_with_source_ref_dict to use storage_class directly (drops DataSource fallback), and replaces the function-scoped storage class fixture with a module-scoped one.
Global storage_class_name_scope_function fixture and golden image wiring
tests/conftest.py, tests/infrastructure/golden_images/test_common_templates_data_volumes.py
Adds storage_class_name_scope_function to the global conftest and updates vm_from_golden_image_multi_storage to consume it when calling data_volume_template_with_source_ref_dict.
Upgrade snapshot tests migrated from Cirros to RHEL VMs
tests/storage/upgrade/constants.py, tests/storage/upgrade/utils.py, tests/storage/upgrade/conftest.py, tests/storage/upgrade/test_upgrade_storage.py
Adds upgrade file name/content constants, rewrites create_vm_for_snapshot_upgrade_tests for RHEL with write_file_via_ssh, replaces Cirros upgrade fixtures with RHEL equivalents, and updates three snapshot-restore test methods with conditional VM stop/start logic.
Predictable-name snapshot restore fixture and test
tests/storage/snapshots/conftest.py, tests/storage/snapshots/test_snapshots.py
Adds source_volume_name_for_predictable_name_restore and vm_restore_with_predictable_names fixtures, and implements test_restore_snapshot_with_predictable_names verifying 63-char truncated deterministic DV/PVC name derivation.

SSH command timeout hardening and miscellaneous fixes

Layer / File(s) Summary
Explicit SSH wait_timeout across virt and test utilities
utilities/virt.py, tests/utils.py, tests/virt/utils.py
Adds wait_timeout=TIMEOUT_2MIN to fetch_pid_from_linux_vm, fetch_pid_from_windows_vm, get_vm_boot_time, get_stress_ng_pid, get_os_cpu_count, and get_os_memory_value; increases run_os_command SSH timeout from 5 to 15.
Descheduler operator reconciliation fixture and pod count adjustment
tests/virt/node/descheduler/conftest.py
Adds descheduler_operator_reconciled fixture (scale-to-0 and back), wires it into both profile fixtures, and changes unallocated_pod_count to target 85% pod capacity with detailed logging.
Dependency bump, image swap, and minor test adjustments
pyproject.toml, tests/infrastructure/instance_types/test_common_vm_preference.py
Bumps openshift-python-wrapper to 11.0.131 and switches start_vm_with_cluster_preference to use Fedora container image instead of RHEL9 guest image.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • RedHatQE/openshift-virtualization-tests#4985: Directly related — updates data_volume_template_with_source_ref_dict to require an explicit storage_class and adjusts vm_from_golden_image_multi_storage to pass the scoped storage class into it.
  • RedHatQE/openshift-virtualization-tests#4962: Directly connected — introduced update_nad_references in lib_helpers.py, which this PR moves to the shared tests/network/libs/nad_ref.py.
  • RedHatQE/openshift-virtualization-tests#5133: Both PRs modify tests/virt/node/descheduler/conftest.py by adding/using the descheduler_operator_reconciled fixture and updating dependent descheduler profile fixtures to take it as a parameter.

Suggested labels

new-tests

Suggested reviewers

  • kgoldbla
  • Ahmad-Hafe
  • dalia-frank
  • EdDev
  • orelmisan
  • vsibirsk
  • rnetser
  • kbidarkar

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 1 inconclusive)

Check name Status Explanation Resolution
Stp Link Required ❌ Error New test function test_restore_snapshot_with_predictable_names in tests/storage/snapshots/test_snapshots.py lacks required STP/RFE/Jira link in its docstring; module and class lack coverage links.... Add STP:/RFE:/Jira: link to test_restore_snapshot_with_predictable_names docstring or to TestRestoreSnapshots class docstring to document the feature/requirement this test validates.
Description check ❓ Inconclusive Description includes all required template sections but lacks details about the upgrade test improvements and omits a Jira ticket URL. Add substantive details about the datasource upgrade test changes and provide the full jira-ticket URL (currently empty).
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main change: replacing Cirros images with RHEL in datasource-based upgrade tests, directly aligning with the primary objective stated in the PR summary.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped RedHatQE/openshift-virtualization-tests-design-docs.


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.

@josemacassan josemacassan changed the base branch from main to cnv-4.22 June 15, 2026 08:07
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@             Coverage Diff             @@
##             cnv-4.22    #5226   +/-   ##
===========================================
  Coverage            ?   98.65%           
===========================================
  Files               ?       25           
  Lines               ?     2459           
  Branches            ?        0           
===========================================
  Hits                ?     2426           
  Misses              ?       33           
  Partials            ?        0           
Flag Coverage Δ
utilities 98.65% <ø> (?)

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

☔ View full report in Codecov by Harness.
📢 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

Copy link
Copy Markdown

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

@openshift-virtualization-qe-bot-3

Copy link
Copy Markdown
Contributor

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: A tracking issue is created for this PR and will be closed when the PR is merged or closed
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: Enabled categories: branch, can-be-merged, cherry-pick, has-conflicts, hold, needs-rebase, size, verified, wip

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
  • /regenerate-welcome - Regenerate this welcome message
  • /security-override - Set security check runs to pass (maintainers only)
  • /security-override cancel - Re-run security checks

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest build-container - Rebuild and test container image
  • /retest verify-bugs-are-open - verify-bugs-are-open
  • /retest all - Run all available tests

Container Operations

  • /build-and-push-container - Build and push container image (tagged with PR number)
    • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3
  • /cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

  • /rebase - Rebase this PR branch onto its base branch

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 2 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
  5. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

  • EdDev
  • dshchedr
  • jpeimer
  • myakove
  • rnetser
  • vsibirsk

Reviewers:

  • Ahmad-Hafe
  • Anatw
  • EdDev
  • OhadRevah
  • RoniKishner
  • SamAlber
  • acinko-rh
  • akri3i
  • albarker-rh
  • azhivovk
  • dalia-frank
  • dshchedr
  • ema-aka-young
  • frenzyfriday
  • geetikakay
  • hmeir
  • josemacassan
  • jpeimer
  • kgoldbla
  • kshvaika
  • mijankow
  • nirdothan
  • orelmisan
  • rlobillo
  • rnetser
  • servolkov
  • vsibirsk
  • yossisegev
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
AI Features
  • Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6[1m])
Security Checks
  • Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
  • Committer Identity Check: Verifies last committer matches PR author
  • Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Container Builds: Container images are automatically tagged with the PR number
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@josemacassan

Copy link
Copy Markdown
Contributor Author

/retest all

@josemacassan

Copy link
Copy Markdown
Contributor Author

/retest tox

@josemacassan josemacassan force-pushed the improve-datasource-upgrade-test-4.20-cnv-4.22 branch from 5204ae2 to a7ae565 Compare June 30, 2026 08:59
@openshift-virtualization-qe-bot-4

Copy link
Copy Markdown

Clean rebase detected — no code changes compared to previous head (5204ae2).

@openshift-virtualization-qe-bot

Copy link
Copy Markdown

/build-and-push-container

@openshift-virtualization-qe-bot-5

Copy link
Copy Markdown

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

@openshift-virtualization-qe-bot

Copy link
Copy Markdown

Verification failed for PR #5226.
Result: UNSTABLE
Job: openshift-virtualization-tests-runner #5844

Execution details
pytest -s -o log_cli=true --jira --skip-deprecated-api-test --upgrade cnv --cnv-version 4.22.0 --cnv-channel=candidate --storage-class-matrix=ocs-storagecluster-ceph-rbd-virtualization --cluster-sanity-skip-check --skip-infra-sanity-check -m 'not
(8 parameters hidden from public report)
Image: openshift-virtualization-tests:pr-5226

@openshift-virtualization-qe-bot

Copy link
Copy Markdown

/build-and-push-container

@openshift-virtualization-qe-bot-4

Copy link
Copy Markdown

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

@openshift-virtualization-qe-bot

Copy link
Copy Markdown

Verification failed for PR #5226.
Result: UNSTABLE
Job: openshift-virtualization-tests-runner #5845

Execution details
pytest -s -o log_cli=true --jira --skip-deprecated-api-test --upgrade cnv --cnv-version 4.22.0 --cnv-channel=candidate --storage-class-matrix=ocs-storagecluster-ceph-rbd-virtualization --cluster-sanity-skip-check --skip-infra-sanity-check -m 'not
(8 parameters hidden from public report)
Image: openshift-virtualization-tests:pr-5226

…rification

- Extract file constants to dedicated constants.py module
- Switch from Cirros to RHEL VM for upgrade tests
- Add explicit file existence checks in snapshot restore test
- Verify first file exists (pre-snapshot) and second file doesn't (post-snapshot)
- Replace cirros-specific run_command with generic run_command_on_vm_and_check_output
- Stop running VM before restore to prevent restore conflicts

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Jose Manuel Castano <joscasta@redhat.com>
@openshift-virtualization-qe-bot

Copy link
Copy Markdown

Clean rebase detected — no code changes compared to previous head (a7ae565).

@openshift-virtualization-qe-bot

Copy link
Copy Markdown

/build-and-push-container

@openshift-virtualization-qe-bot-2

Copy link
Copy Markdown
Contributor

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

@openshift-virtualization-qe-bot

Copy link
Copy Markdown

Verification failed for PR #5226.
Result: UNSTABLE
Job: openshift-virtualization-tests-runner #5855

Execution details
pytest -s -o log_cli=true --jira --skip-deprecated-api-test --upgrade cnv --cnv-version 4.22.0 --cnv-channel=stable --storage-class-matrix=ocs-storagecluster-ceph-rbd-virtualization --cluster-sanity-skip-check --skip-infra-sanity-check -m 'not
(8 parameters hidden from public report)
Image: openshift-virtualization-tests:pr-5226

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.

7 participants