Open
Conversation
|
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. |
Contributor
📝 WalkthroughWalkthroughThis PR introduces dual-stack IPv4/IPv6 support to network localnet tests by adding IP utility functions, refactoring fixture signatures to support IPv6 address pools and cluster flags, and updating test logic to validate connectivity and configuration per destination address rather than per VM. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Rationale: Multiple heterogeneous changes across 6 files with substantial structural refactoring (fixture signatures, constants renamed, behavioral logic rewritten). Key concerns include: (1) fixture signature proliferation with new parameters across conftest.py requiring validation of all propagations, (2) per-address test logic patterns introduced across 3 test files demanding review of subtest correctness and IP family handling, (3) removal of legacy fixtures requiring confirmation of all callsites updated, (4) new utility functions ( 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/network/libs/ip.py`:
- Around line 98-99: Add a Google-style docstring to the public helper function
ip_header_size that describes accepted input types (ipaddress.IPv4Address or
ipaddress.IPv6Address), explains behavior (returns IPV4_HEADER_SIZE for IPv4 and
IPV6_HEADER_SIZE for IPv6), and states the return units (bytes) and type (int);
place the docstring immediately under the def ip_header_size(...) line and
include brief examples or edge-case notes if needed (e.g., what happens for
unexpected types).
In `@tests/network/localnet/liblocalnet.py`:
- Around line 35-47: The function random_ip_addresses currently uses boolean
flags (ipv4_supported_cluster, ipv6_supported_cluster) which triggers Ruff
FBT001 and hides intent; change the signature to accept a clear collection of
supported protocols (e.g., supported_protocols: Iterable[str] or an Enum like
Protocol.{IPv4,IPv6}) instead of boolean parameters, update the body to check
membership (e.g., if "ipv4" in supported_protocols:
addresses.append(next(ipv4_localnet_address_pool))) and update all callers to
pass the protocol list/enum; also add a Google-format docstring to
random_ip_addresses that documents the generator side effect (it advances
ipv4_localnet_address_pool and ipv6_localnet_address_pool via next()) and
precise return value, and tighten the type hints for the generators to
Generator[str, None, None].
In `@tests/network/localnet/test_default_bridge.py`:
- Around line 46-56: The loop over destinations can be vacuous if
filter_link_local_addresses returns no non-link-local addresses; modify the test
in tests/network/localnet/test_default_bridge.py to assert there is at least one
address before entering the for loop (use the result of
filter_link_local_addresses(ip_addresses=iface.ipAddresses) and fail the test if
empty), then proceed with the existing per-address subtests using
client_server_active_connection with migrated_localnet_vm, base_localnet_vm,
LOCALNET_BR_EX_INTERFACE, port 8888 and ip_family=dst_ip.version; reference
lookup_iface_status and is_tcp_connection unchanged.
- Around line 78-82: The test compares ordered lists reported_ips and
expected_ips but address ordering across families can vary; update the assertion
to compare unordered sets instead (e.g., convert reported_ips and expected_ips
into sets of canonical values such as their string representations or
ip_interface objects) so the quarantined dual-stack check passes regardless of
IPv4/IPv6 ordering; locate the code building reported_ips via
filter_link_local_addresses(ip_interface(...)) and the expected_ips from
vm_localnet_1_no_vlan_iface_addresses and change the final assert to compare
set(reported_ips) == set(expected_ips).
In `@tests/network/localnet/test_ovs_bridge.py`:
- Around line 47-55: The test loop over filter_link_local_addresses can be
vacuously skipped if it returns an empty list, causing false positives; before
iterating, capture the filtered list (e.g., dst_ips =
list(filter_link_local_addresses(ip_addresses=iface.ipAddresses)) ) and assert
or fail the test if dst_ips is empty (use the test harness failure method in
this file, e.g., self.fail or an assert), otherwise iterate over dst_ips in the
existing for-loop that calls subtests.test and client_server_active_connection
(referencing filter_link_local_addresses, iface.ipAddresses, subtests.test,
client_server_active_connection, and LOCALNET_OVS_BRIDGE_INTERFACE).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
tests/network/libs/ip.pytests/network/localnet/conftest.pytests/network/localnet/liblocalnet.pytests/network/localnet/test_default_bridge.pytests/network/localnet/test_jumbo_frames.pytests/network/localnet/test_ovs_bridge.py
Contributor
Author
|
/wip |
openshift-virtualization-qe-bot-2
changed the title
Localnet tests need adjustments to run on IPv6 single stack clusters: - Add random IPv6 address to vlan and non-vlan interface on tested VM - Run extra iperf3 server to test IPv6 tcp connectivity test_vmi_reports_ip_on_secondary_interface_without_vlan test was also adjusted but quarantined due to a bug in which the requested assigned IP addresses on the tested VM are not visible in VMI status Signed-off-by: Asia Khromov <azhivovk@redhat.com>
azhivovk
force-pushed
the
adjust_ipv6_localnet
branch
from
a14f418 to
b5c68d9
Compare
Contributor
Author
|
Contributor
Author
|
/wip cancel |
openshift-virtualization-qe-bot-4
changed the title
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Localnet tests need adjustments to run on
IPv6 single stack clusters:
test_vmi_reports_ip_on_secondary_interface_without_vlan test was also adjusted but quarantined due to a bug in which the requested assigned IP addresses on the tested VM are not visible in VMI status
jira-ticket: https://issues.redhat.com/browse/CNV-78445
Summary by CodeRabbit
Tests
Refactor