Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update assertion for discovery plugin test as per IPv4/Ipv6 ip address #17633

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amolpati30
Copy link
Contributor

Previously, there was only one column for IP Address. However, starting from version 6.17, both IPv4 and IPv6 networks are used. When a discovery host is created, two separate columns are displayed for IPv4 and IPv6. To prevent assertion failures, the IP address will be fetched based on the respective network.

Prior - image

Latest - image

@amolpati30 amolpati30 added Easy Fix :) Easiest Fix to review and quick merge request. No-CherryPick PR doesnt need CherryPick to previous branches Stream Introduced in or relating directly to Satellite Stream/Master labels Feb 19, 2025
@amolpati30 amolpati30 requested a review from a team as a code owner February 19, 2025 10:55
@amolpati30 amolpati30 force-pushed the update_assertion_as_per_IPv4/Ipv6 branch from 06ed385 to 2020174 Compare February 19, 2025 11:00
@@ -236,7 +236,10 @@ def _finalize():
values = session.discoveryrule.read_discovered_hosts(discovery_rule.name)
assert values['searchbox'] == rule_search
assert len(values['table']) == 1
assert values['table'][0]['IP Address'] == ip_address
if values['table'][0]['IPv4'] != '':
Copy link
Contributor

Choose a reason for hiding this comment

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

We can assert the values based on server.is_ipv6 but this won't work when we will have dual stack network so I would suggest to assert both without if else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When a dual-stack network is available, we can take the appropriate action accordingly.
When creating a discovery host, there are two columns for IPv4 and IPv6. The values are fetched from these columns and matched with ip_address = gen_ipaddr(). I believe we need to update this section ip_address = gen_ipaddr() to ensure the assertion works with dual-stack later.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that but you would still need to check which address to read (ipv4 or ipv6) from UI instead of values['table'][0]['IP Address'].
gen_ipaddr() already takes care of which ip address to pick.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have an example for reference?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you try assert values['table'][0]['IPv6' if settings.server.is_ipv6 else 'IPv4'] == ip_address ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@amolpati30 I don't think there is any reference in robottelo currently but you can check based on is_ipv6.

@amolpati30 amolpati30 force-pushed the update_assertion_as_per_IPv4/Ipv6 branch 2 times, most recently from 7de2b5a to 8ae932a Compare February 19, 2025 13:08
@amolpati30 amolpati30 requested a review from shweta83 February 19, 2025 16:06
@amolpati30 amolpati30 force-pushed the update_assertion_as_per_IPv4/Ipv6 branch 3 times, most recently from 5917a4b to cff307e Compare February 20, 2025 04:51
@amolpati30 amolpati30 force-pushed the update_assertion_as_per_IPv4/Ipv6 branch from cff307e to 8dc923c Compare February 20, 2025 05:00
@amolpati30
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_discoveryrule.py -k test_positive_list_host_based_on_rule_search_query

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 10256
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_discoveryrule.py -k test_positive_list_host_based_on_rule_search_query --external-logging
Test Result : =========== 1 passed, 3 deselected, 17 warnings in 663.49s (0:11:03) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Feb 20, 2025
@Gauravtalreja1
Copy link
Collaborator

trigger: test-robottelo
pytest: tests/foreman/ui/test_discoveryrule.py -k test_positive_list_host_based_on_rule_search_query
network_type: ipv6

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 10257
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_discoveryrule.py -k test_positive_list_host_based_on_rule_search_query --external-logging
Test Result : ====== 1 failed, 3 deselected, 15 warnings, 1 error in 621.48s (0:10:21) =======

@Satellite-QE Satellite-QE added PRT-Failed Indicates that latest PRT run is failed for the PR and removed PRT-Passed Indicates that latest PRT run is passed for the PR labels Feb 20, 2025
@Gauravtalreja1 Gauravtalreja1 added CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.16.z Introduced in or relating directly to Satellite 6.16 and removed No-CherryPick PR doesnt need CherryPick to previous branches Stream Introduced in or relating directly to Satellite Stream/Master labels Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.16.z Introduced in or relating directly to Satellite 6.16 6.17.z AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing CherryPick PR needs CherryPick to previous branches Easy Fix :) Easiest Fix to review and quick merge request. PRT-Failed Indicates that latest PRT run is failed for the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants