Modify windows vm deployment to include virtio#3303
Conversation
modify the fixture for creating windows vm to include virtio and wsl
📝 WalkthroughWalkthroughNetwork metrics test utilities now return parsed packet stats as dicts and accept precomputed packet data for comparisons; Windows WSL parsing and provisioning switched to registry-based images with updated sizes/instance type and dv wait timeout; provisioning cleanup moved to a finally block. New fixtures expose packet-data for Linux and Windows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-3303 published |
Windows vm needs to include virtio drivers in order some metrics will be able to get the info they need. Also updated network metrics to work with the new change and simplified it.
d970ce6
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Your deliverable: Test Execution Plan
Guidelines:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 979 🏁 Script executed: Length of output: 1728 🧠 Learnings used |
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-3303 published |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/observability/metrics/test_network_metrics.py (1)
15-26: MEDIUM: Remove unused fixture params; use@pytest.mark.usefixturesinstead.
linux_vm_for_test_interface_nameandgenerated_network_trafficare not used in the body. Per test guidelines, move them tousefixturesto avoid unused args and make intent explicit.♻️ Proposed fix
-@pytest.mark.usefixtures("vm_for_test", "linux_vm_for_test_interface_name") +@pytest.mark.usefixtures("vm_for_test", "linux_vm_for_test_interface_name", "generated_network_traffic") class TestVmiNetworkMetricsLinux: @@ def test_kubevirt_vmi_network_traffic_bytes_total( self, prometheus, vm_for_test, - linux_vm_for_test_interface_name, - generated_network_traffic, network_packet_received_linux_vm, ):As per coding guidelines, use
@pytest.mark.usefixtureswhen fixture return values are unused.
🤖 Fix all issues with AI agents
In `@tests/observability/metrics/test_network_metrics.py`:
- Around line 34-44: The test method
TestVmiNetworkMetricsWindows.test_kubevirt_vmi_network_traffic_bytes_total_windows_vm
currently declares unused fixture parameters windows_vm_for_test_interface_name
and generated_network_traffic_windows_vm; remove those two parameters from the
test signature and instead ensure the fixtures still run by adding
`@pytest.mark.usefixtures`("windows_vm_for_test_interface_name",
"generated_network_traffic_windows_vm") above the test (or on the
TestVmiNetworkMetricsWindows class) so the fixtures execute but their return
values are not required as arguments.
|
/verified Executed all observability windows tests, build 4286 |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Your deliverable: Test Execution Plan
Guidelines:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 581 🏁 Script executed: Length of output: 1735 🧠 Learnings used |
|
/lgtm |
|
/approve |
|
Successfully removed PR tag: quay.io/openshift-cnv/openshift-virtualization-tests:pr-3303. |
|
/cherry-pick cnv-4.20 |
|
Manual cherry-pick is needed |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
|
/cherry-pick cnv-4.21 |
|
Cherry-picked PR Modify windows vm deployment to include virtio into cnv-4.21 |
Short description:
modify the fixture for creating windows vm to include virtio and wsl
More details:
What this PR does / why we need it:
This PR updates the windows vm deployment to include virtio drivers, we need it because there is metrics that not reporting any value if the virtio not installed.
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://issues.redhat.com/browse/CNV-74764
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.