Skip to content

Conversation

@vsibirsk
Copy link
Collaborator

@vsibirsk vsibirsk commented Nov 30, 2025

Short description:
  • Root cause fix is in python-wrapper (full details in jira ticket)
  • Refactor pause/unpause function names
  • drop optional migraion (never used)
  • remove admin_client dependency
More details:

Actual fix is in python-wrapper: RedHatQE/openshift-python-wrapper#2596

What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:

https://issues.redhat.com/browse/CNV-72168

Summary by CodeRabbit

  • Refactor

    • Simplified VM pause/unpause utilities by removing optional migration paths and renaming helper functions for clearer, more consistent behavior.
  • Tests

    • Updated tests to use the streamlined utilities across OS and GPU scenarios; removed expected-failure markers so previously quarantined pause/unpause tests now run as active tests.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 30, 2025

Walkthrough

Multiple pause/unpause helpers were renamed to remove "optional_migrate" and migration parameters were removed; tests and utilities were updated to call the new functions and RHEL test imports/xfail decorators were adjusted.

Changes

Cohort / File(s) Summary
Core utilities & exports
utilities/virt.py, tests/virt/utils.py
Renamed pause_optional_migrate_unpause_and_check_connectivitypause_unpause_vm_and_check_connectivity. Renamed validate_pause_optional_migrate_unpause_linux_vmvalidate_pause_unpause_linux_vm and validate_pause_optional_migrate_unpause_windows_vmvalidate_pause_unpause_windows_vm. Removed migrate parameter and simplified connectivity checks (now use SSH wait).
Linux OS support tests
tests/infrastructure/instance_types/supported_os/test_rhel_os.py, tests/virt/cluster/common_templates/.../centos/test_centos_os_support.py, tests/virt/cluster/common_templates/.../fedora/test_fedora_os_support.py, tests/virt/cluster/common_templates/.../rhel/test_rhel_os_support.py
Replaced imports/call sites from validate_pause_optional_migrate_unpause_linux_vm to validate_pause_unpause_linux_vm. In RHEL test removed QUARANTINED import and removed @xfail from pause/unpause tests.
Windows OS support tests
tests/virt/cluster/common_templates/windows/test_windows_os_support.py, tests/virt/node/gpu/.../test_windows_vm_with_gpu_pci_passthrough.py, tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
Replaced validate_pause_optional_migrate_unpause_windows_vm with validate_pause_unpause_windows_vm in imports and call sites (including pre_pause_pid usage).
GPU / vGPU tests
tests/virt/node/gpu/gpu_pci_passthrough/test_rhel_vm_with_gpu_pci_passthrough.py, tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py
Replaced pause_optional_migrate_unpause_and_check_connectivity calls/imports with pause_unpause_vm_and_check_connectivity.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify consistent renames across utilities and all test call sites.
  • Confirm removal of migrate parameter and changed connectivity checks do not break callers that expected migration behavior.
  • Check RHEL test changes (removed QUARANTINED import and xfail) to ensure tests are stable and intended to run.

Possibly related PRs

Suggested labels

verified, can-be-merged

Suggested reviewers

  • RoniKishner
  • kbidarkar
  • rnetser
  • dshchedr
  • jerry7z
  • akri3i

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive The description includes most required sections: short description with key points, reference to related python-wrapper PR, and jira ticket link. However, 'What this PR does / why we need it', 'Which issue(s) this PR fixes', and 'Special notes for reviewer' sections are empty, leaving context gaps. Complete the empty template sections, particularly 'What this PR does / why we need it' and 'Which issue(s) this PR fixes' to provide comprehensive context for reviewers.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'De-quarantine: test_pause_unpause_vm' clearly summarizes the main objective: removing expected failure markers from pause/unpause VM tests.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa8b068 and e7de610.

📒 Files selected for processing (11)
  • tests/infrastructure/instance_types/supported_os/test_rhel_os.py (2 hunks)
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py (3 hunks)
  • tests/virt/cluster/common_templates/fedora/test_fedora_os_support.py (3 hunks)
  • tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py (4 hunks)
  • tests/virt/cluster/common_templates/windows/test_windows_os_support.py (3 hunks)
  • tests/virt/node/gpu/gpu_pci_passthrough/test_rhel_vm_with_gpu_pci_passthrough.py (3 hunks)
  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py (3 hunks)
  • tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py (2 hunks)
  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py (2 hunks)
  • tests/virt/utils.py (2 hunks)
  • utilities/virt.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • tests/virt/cluster/common_templates/fedora/test_fedora_os_support.py
  • tests/virt/cluster/common_templates/windows/test_windows_os_support.py
  • tests/infrastructure/instance_types/supported_os/test_rhel_os.py
  • tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py
  • tests/virt/node/gpu/gpu_pci_passthrough/test_rhel_vm_with_gpu_pci_passthrough.py
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
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: 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: 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: 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: 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: 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.
📚 Learning: 2025-06-18T09:19:05.769Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/test_vms_metrics.py:129-137
Timestamp: 2025-06-18T09:19:05.769Z
Learning: For Windows VM testing in tests/observability/metrics/test_vms_metrics.py, it's acceptable to have more fixture parameters than typical pylint recommendations when reusing expensive Windows VM fixtures for performance. Windows VMs take a long time to deploy, so reusing fixtures like windows_vm_for_test and adding labels via windows_vm_with_low_bandwidth_migration_policy is preferred over creating separate fixtures that would require additional VM deployments.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py
  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
  • tests/virt/utils.py
📚 Learning: 2025-06-18T09:21:34.315Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:21:34.315Z
Learning: In tests/observability/metrics/conftest.py, when creating fixtures that modify shared Windows VM state (like changing nodeSelector), prefer using function scope rather than class scope to ensure ResourceEditor context managers properly restore the VM state after each test, maintaining test isolation while still reusing expensive Windows VM fixtures.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/utils.py
📚 Learning: 2025-09-03T07:23:37.045Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:30-31
Timestamp: 2025-09-03T07:23:37.045Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, test_vm_deletion should depend on test_create_vm (not test_start_vm) to ensure resource cleanup happens even if the VM fails to start, preventing resource leaks in the test environment.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
  • tests/virt/utils.py
📚 Learning: 2025-09-29T20:33:51.007Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:65-72
Timestamp: 2025-09-29T20:33:51.007Z
Learning: In tests/virt/node/migration_and_maintenance/conftest.py, the added_vm_cpu_limit fixture doesn't require ResourceEditor as a context manager because it's the final test to modify the vm_for_multifd_test VM before teardown, so restoration of CPU limits is unnecessary overhead as confirmed by maintainer dshchedr.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
  • tests/virt/utils.py
📚 Learning: 2025-05-26T13:36:16.136Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 584
File: tests/observability/metrics/conftest.py:1136-1136
Timestamp: 2025-05-26T13:36:16.136Z
Learning: For Windows VM testing in metrics tests, the `verify_wsl2_guest_works(vm=vm)` call in the `windows_vm_for_test` fixture uses a 60-second timeout as a deliberate "fail fast" validation. This upfront check prevents more expensive failures later since Windows VM deployment is time-consuming, and WSL2 functionality is critical for the Windows VM tests to work properly.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
  • utilities/virt.py
  • tests/virt/utils.py
📚 Learning: 2025-09-03T07:22:50.446Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:27-28
Timestamp: 2025-09-03T07:22:50.446Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, Windows guest images are configured with SSH support (likely through WSL2), so running_vm() should keep the default check_ssh_connectivity=True to verify SSH connectivity is working properly in the Windows guest images.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py
  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
  • utilities/virt.py
  • tests/virt/utils.py
📚 Learning: 2025-08-14T10:28:22.958Z
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 1785
File: tests/virt/cluster/common_templates/utils.py:65-66
Timestamp: 2025-08-14T10:28:22.958Z
Learning: The restart_qemu_guest_agent_service function in tests/virt/cluster/common_templates/utils.py is only called for RHEL7 VMs, with OS version checks handled by the calling test code, not within the function itself. Guest agent functionality is verified by subsequent tests in the test class.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py
  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
  • utilities/virt.py
📚 Learning: 2025-08-04T15:27:14.175Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1082
Timestamp: 2025-08-04T15:27:14.175Z
Learning: In tests/observability/metrics/conftest.py, the `non_existent_node_windows_vm` fixture is used for tier3 Windows VM testing and must use Windows VMs rather than Linux VMs, even though it adds overhead, because it's specifically testing Windows VM status transition metrics as part of dedicated Windows VM test coverage.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
📚 Learning: 2025-06-19T09:56:06.185Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1244
File: tests/virt/cluster/common_templates/windows/test_windows_custom_options.py:181-181
Timestamp: 2025-06-19T09:56:06.185Z
Learning: In tests/virt/cluster/common_templates/windows/test_windows_custom_options.py, the change from os.path.join to f-string path construction is intentional to handle cases where Images.Windows.WIN2k19_HA_IMG may be None during test collection on architectures where Windows images aren't available. The f-string approach allows pytest collection to succeed gracefully, while actual test execution is controlled by markers and environmental conditions.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
📚 Learning: 2025-06-22T13:47:35.014Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1183-1186
Timestamp: 2025-06-22T13:47:35.014Z
Learning: In tests/observability/metrics/conftest.py, the `stopped_windows_vm` fixture is designed to temporarily stop the Windows VM for a test, then restart it during teardown (after yield) because the Windows VM is module-scoped and needs to be available for other tests that depend on it being in a running state.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
  • tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py
  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
  • tests/virt/utils.py
📚 Learning: 2025-05-27T11:44:14.859Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 584
File: tests/observability/metrics/test_network_metrics.py:62-76
Timestamp: 2025-05-27T11:44:14.859Z
Learning: The windows_vm_for_test fixture in tests/observability/metrics/conftest.py does not have a request argument, so it cannot be parametrized using pytest.mark.parametrize with indirect=True. This is different from vm_for_test fixture which accepts parameters through parametrization.

Applied to files:

  • tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py
📚 Learning: 2025-09-17T14:02:24.619Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1702
File: tests/chaos/oadp/conftest.py:23-45
Timestamp: 2025-09-17T14:02:24.619Z
Learning: In the openshift-virtualization-tests repository, VirtualMachineForTests class automatically generates unique VM names when generate_unique_name=True (default). It uses utilities.infra.unique_name() which appends a timestamp to create names like "vm-oadp-chaos-{timestamp}", so hardcoded VM names in fixtures don't cause collisions during parallel test execution.

Applied to files:

  • tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py
📚 Learning: 2025-10-30T10:43:48.886Z
Learnt from: josemacassan
Repo: RedHatQE/openshift-virtualization-tests PR: 2300
File: tests/storage/online_resize/conftest.py:94-96
Timestamp: 2025-10-30T10:43:48.886Z
Learning: In tests/storage/online_resize/conftest.py, the `running_rhel_vm` fixture is intentionally designed to only invoke `running_vm(vm=rhel_vm_for_online_resize)` without returning or yielding a value. It's used as a dependency fixture to ensure the VM is running before other fixtures/tests execute, while the actual VM object is accessed via the `rhel_vm_for_online_resize` fixture.

Applied to files:

  • tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
📚 Learning: 2025-05-18T09:24:43.335Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 990
File: utilities/virt.py:1704-1710
Timestamp: 2025-05-18T09:24:43.335Z
Learning: The `migrate_vm_and_verify` function in utilities/virt.py has inconsistent return behavior - it returns a VirtualMachineInstanceMigration object when wait_for_migration_success=False and returns None when wait_for_migration_success=True. This has been properly typed but should be refactored in a future PR for better API design.

Applied to files:

  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
  • utilities/virt.py
  • tests/virt/utils.py
📚 Learning: 2025-06-13T01:08:18.579Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 890
File: tests/virt/node/descheduler/conftest.py:146-148
Timestamp: 2025-06-13T01:08:18.579Z
Learning: The fixture `vms_orig_nodes_before_node_drain` in tests/virt/node/descheduler/conftest.py is intentionally kept with the “node_drain” suffix because it represents the state directly before a node drain step; future refactor suggestions should preserve this name unless requirements change.

Applied to files:

  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
📚 Learning: 2025-09-14T05:39:47.969Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/conftest.py:117-131
Timestamp: 2025-09-14T05:39:47.969Z
Learning: In utilities/virt.py, Windows OS detection logic uses substring containment checks (e.g., `OS_FLAVOR_WINDOWS in self.os_flavor`) instead of exact equality, allowing os_flavor values like "win-container-disk" to correctly trigger Windows-specific configurations for memory, CPU, SSH authentication, and cloud-init settings.

Applied to files:

  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
📚 Learning: 2025-09-14T05:39:47.969Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/conftest.py:117-131
Timestamp: 2025-09-14T05:39:47.969Z
Learning: In utilities/virt.py, the Windows OS detection logic uses substring matching (e.g., checking if "win" is contained in os_flavor) rather than exact equality, allowing os_flavor values like "win-container-disk" to correctly trigger Windows-specific configurations for memory, CPU, SSH authentication, and cloud-init settings.

Applied to files:

  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
📚 Learning: 2025-05-28T09:58:09.482Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 983
File: tests/storage/cdi_clone/test_clone.py:158-160
Timestamp: 2025-05-28T09:58:09.482Z
Learning: In the OpenShift Virtualization tests codebase, using `.get()` without defaults when accessing OS configuration dictionaries (like WINDOWS_11) is acceptable during pytest collection phase because tests are filtered based on architecture and OS configuration presence during actual test execution. Tests won't run if the required OS configuration isn't available for the target architecture.

Applied to files:

  • tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py
📚 Learning: 2025-08-06T13:57:34.740Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1257-1258
Timestamp: 2025-08-06T13:57:34.740Z
Learning: In the openshift-virtualization-tests repository, all existing calls to the `get_cluster_platform` function in utilities/infra.py already pass the `admin_client` parameter, so the breaking change requiring this parameter does not actually break any existing code.

Applied to files:

  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
📚 Learning: 2025-06-23T19:24:28.327Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/test_mass_machine_type_transition.py:97-104
Timestamp: 2025-06-23T19:24:28.327Z
Learning: In OpenShift Virtualization machine type transition tests, the test_machine_type_transition_without_restart method with restart_required=false parameter validates that VM machine types do NOT change when the lifecycle job runs with restart disabled, so the assertion should check against the original machine type rather than the target machine type.

Applied to files:

  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
📚 Learning: 2025-06-23T19:18:12.275Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:142-149
Timestamp: 2025-06-23T19:18:12.275Z
Learning: In OpenShift Virtualization machine type transition tests, the kubevirt_api_lifecycle_automation_job updates VM machine types to the latest version based on a MACHINE_TYPE_GLOB pattern, and subsequent fixtures may intentionally revert the machine type to test bidirectional transition behavior.

Applied to files:

  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
📚 Learning: 2025-08-25T08:59:47.233Z
Learnt from: SiboWang1997
Repo: RedHatQE/openshift-virtualization-tests PR: 1669
File: utilities/virt.py:2645-2649
Timestamp: 2025-08-25T08:59:47.233Z
Learning: In OpenShift Virtualization tests with Windows OS matrix, Windows VM reboots reach the "Running" state within 10 seconds, making the default wait_until_running_timeout of 4 minutes more than sufficient for guest reboot scenarios in utilities/virt.py.

Applied to files:

  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
📚 Learning: 2025-06-18T09:15:25.436Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1178-1180
Timestamp: 2025-06-18T09:15:25.436Z
Learning: In tests/observability/metrics/conftest.py, the `stopped_vm_metric_1` fixture is intentionally designed to stop the VM and leave it in that state - it does not need to restart the VM afterward as this is the desired behavior for the tests that use it.

Applied to files:

  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: For PR #1904 test execution, the critical validation point is test_connectivity_over_migration_between_localnet_vms which should fail gracefully on cloud clusters but pass on bare-metal/PSI clusters, representing the core nmstate conditional logic functionality.

Applied to files:

  • tests/virt/cluster/common_templates/centos/test_centos_os_support.py
  • utilities/virt.py
🧬 Code graph analysis (5)
tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py (2)
tests/virt/utils.py (2)
  • validate_pause_unpause_windows_vm (286-295)
  • verify_gpu_device_exists_in_vm (398-411)
tests/virt/node/gpu/conftest.py (1)
  • gpu_vma (14-37)
tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py (2)
utilities/virt.py (1)
  • pause_unpause_vm_and_check_connectivity (2467-2471)
tests/virt/node/gpu/conftest.py (1)
  • gpu_vma (14-37)
tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py (1)
tests/virt/utils.py (1)
  • validate_pause_unpause_windows_vm (286-295)
tests/virt/cluster/common_templates/centos/test_centos_os_support.py (1)
utilities/virt.py (1)
  • validate_pause_unpause_linux_vm (2474-2483)
tests/virt/utils.py (1)
utilities/virt.py (2)
  • pause_unpause_vm_and_check_connectivity (2467-2471)
  • VirtualMachineForTests (203-1106)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: can-be-merged
  • GitHub Check: can-be-merged
  • GitHub Check: can-be-merged
  • GitHub Check: build-container
  • GitHub Check: tox
  • GitHub Check: can-be-merged
  • GitHub Check: can-be-merged
🔇 Additional comments (7)
tests/virt/cluster/common_templates/centos/test_centos_os_support.py (1)

24-24: LGTM! Consistent refactoring of pause/unpause utilities.

The import and call sites have been correctly updated to use the renamed validate_pause_unpause_linux_vm function. The function signature remains compatible (both calls correctly pass vm and optionally pre_pause_pid), and the removal of the unused migrate parameter simplifies the API.

Also applies to: 120-120, 135-135

utilities/virt.py (2)

2467-2471: Improved post-unpause verification.

The new pause_unpause_vm_and_check_connectivity function simplifies the API by removing the unused migrate parameter and improves robustness by verifying SSH connectivity after unpause rather than just checking if the VM is running. This ensures the VM is truly functional and accessible after the pause/unpause cycle.


2474-2483: LGTM! Clean refactoring with improved type safety.

The function rename from validate_pause_optional_migrate_unpause_linux_vm to validate_pause_unpause_linux_vm is clearer and more concise. The addition of type hints (vm: VirtualMachineForTests, pre_pause_pid: int | None = None) improves code quality and IDE support. The implementation correctly delegates to the new pause_unpause_vm_and_check_connectivity helper.

tests/virt/node/gpu/vgpu/test_rhel_vm_with_vgpu.py (1)

30-30: LGTM! Consistent update for vGPU test.

The import and call site correctly use the renamed pause_unpause_vm_and_check_connectivity function. The test logic is preserved - the VM is paused/unpaused within the running_sleep_in_linux context manager to verify process continuity.

Also applies to: 155-155

tests/virt/node/gpu/gpu_pci_passthrough/test_windows_vm_with_gpu_pci_passthrough.py (1)

14-14: LGTM! Consistent Windows VM pause/unpause updates.

The import and both call sites correctly use the renamed validate_pause_unpause_windows_vm function. Both PCI passthrough tests (hostdevices spec and gpus spec) now use the simplified API without the unused migrate parameter.

Also applies to: 75-75, 99-99

tests/virt/utils.py (1)

61-61: LGTM! Windows VM helper consistently refactored.

The validate_pause_unpause_windows_vm function has been successfully refactored to match the Linux VM helper pattern. The function name is clearer, the unused migrate parameter has been removed, type hints have been added, and it correctly delegates to the shared pause_unpause_vm_and_check_connectivity helper. The Windows-specific PID verification logic is preserved.

Also applies to: 286-295

tests/virt/node/gpu/vgpu/test_windows_vm_with_vgpu.py (1)

18-18: LGTM! Consistent vGPU Windows test update.

The import and call site correctly use the renamed validate_pause_unpause_windows_vm function. The Windows 10 vGPU test now uses the simplified API consistent with other GPU and PCI passthrough tests.

Also applies to: 105-105


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.

@openshift-virtualization-qe-bot-2
Copy link
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
  • Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified

📋 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)

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and 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 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

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, or conflict labels
  5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

  • dshchedr
  • myakove
  • rnetser
  • vsibirsk

Reviewers:

  • RoniKishner
  • SamAlber
  • SiboWang1997
  • akri3i
  • dshchedr
  • geetikakay
  • jerry7z
  • kbidarkar
  • rnetser
  • vsibirsk
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is automatically removed on each new commit
  • 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.

@codecov
Copy link

codecov bot commented Nov 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.52%. Comparing base (3bc5556) to head (e7de610).
⚠️ Report is 63 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2926      +/-   ##
==========================================
+ Coverage   92.81%   96.52%   +3.71%     
==========================================
  Files          18       22       +4     
  Lines        1447     1613     +166     
==========================================
+ Hits         1343     1557     +214     
+ Misses        104       56      -48     
Flag Coverage Δ
utilities 96.52% <ø> (+3.71%) ⬆️

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.

 - Root cause fix is in python-wrapper
 - Refactor pause/unpause function names
 - drop optional migaraion (never used)
@SamAlber
Copy link
Contributor

/lgtm

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.

6 participants