[6.18.z] Fix Discovery provisioning tests - #21062
Closed
Satellite-QE wants to merge 1 commit into
Closed
Conversation
Satellite-QE
added
6.18.z
Collaborator
Author
|
Satellite-QE
added
the
AutoMerge_Cherry_Picked
Contributor
Reviewer's GuideThis PR stabilizes discovery provisioning tests by cleaning up discovered hosts during provisioning setup and conditionally reassigning organization/location for discovered hosts in UI tests when a known bug is open, while leaving API tests to rely on automatic assignment and not forcing hostgroup/build via the API. Sequence diagram for discovery provisioning fixture with discovered host cleanupsequenceDiagram
actor Tester
participant Pytest as pytest_runner
participant Fixture as module_provisioning_sat
participant Sat as Satellite
participant DHCPD as dhcpd_service
Tester ->> Pytest: run discovery_provisioning_tests
Pytest ->> Fixture: module_provisioning_sat(provisioning_type=discovery)
Fixture ->> Sat: configure_provisioning_resources()
Sat -->> Fixture: provisioning_resources_configured
rect rgb(230,230,250)
Fixture ->> Sat: api.DiscoveredHost().search()
Sat -->> Fixture: discovered_hosts_list
loop for_each_discovered_host
Fixture ->> Sat: host.delete()
Sat -->> Fixture: host_deleted
end
end
alt network_type_is_IPV4
Fixture ->> Sat: execute(cat_null_dhcpd_leases)
Sat -->> Fixture: status_0
Fixture ->> Sat: execute(systemctl_restart_dhcpd)
Sat ->> DHCPD: restart()
DHCPD -->> Sat: restarted
Sat -->> Fixture: status_0
end
Fixture -->> Pytest: ready_for_discovery_provisioning
Pytest -->> Tester: run_tests_with_clean_discovery_state
Class diagram for Satellite discovery API usage in provisioning and testsclassDiagram
class Satellite {
+NetworkType network_type
+API api
+CommandResult execute(command)
}
class API {
+DiscoveredHost DiscoveredHost()
}
class DiscoveredHost {
+int id
+string name
+string organization
+string location
+string hostgroup
+bool build
+list~DiscoveredHost~ search()
+void delete()
+void assign_organization(organization)
+void assign_location(location)
+void set_hostgroup(hostgroup)
+void set_build(build)
+void save()
}
class CommandResult {
+int status
+string stdout
+string stderr
}
class TestDiscoveredHostAPI {
+void test_discovered_host_provisioning_uses_auto_assignment()
}
class TestDiscoveredHostUI {
+void test_discovered_host_provisioning_with_conditional_reassignment()
}
Satellite --> API : has
API --> DiscoveredHost : creates
Satellite --> CommandResult : returns
TestDiscoveredHostAPI --> Satellite : uses_api
TestDiscoveredHostUI --> DiscoveredHost : reassigns_org_location_conditionally
TestDiscoveredHostUI --> Satellite : uses_ui_and_api
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
is_open('SAT-33477')UI-based reassignment logic for discovered hosts is duplicated in multiple tests; consider extracting this into a small helper to keep the tests DRY and make the workaround easier to remove once the bug is closed. - In
module_provisioning_sat, deleting allDiscoveredHostrecords whenprovisioning_type == 'discovery'is quite broad; if possible, scope the cleanup (e.g., by org/location or a naming convention) to avoid unintended removal of unrelated discovered hosts used by other tests.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `is_open('SAT-33477')` UI-based reassignment logic for discovered hosts is duplicated in multiple tests; consider extracting this into a small helper to keep the tests DRY and make the workaround easier to remove once the bug is closed.
- In `module_provisioning_sat`, deleting all `DiscoveredHost` records when `provisioning_type == 'discovery'` is quite broad; if possible, scope the cleanup (e.g., by org/location or a naming convention) to avoid unintended removal of unrelated discovered hosts used by other tests.
## Individual Comments
### Comment 1
<location path="pytest_fixtures/component/provision_pxe.py" line_range="214-216" />
<code_context>
remote_execution_proxy=[module_provisioning_capsule.id],
domain=[domain.id],
).create()
+ if provisioning_type == 'discovery':
+ for host in sat.api.DiscoveredHost().search():
+ host.delete()
+
if sat.network_type == NetworkType.IPV4:
</code_context>
<issue_to_address>
**issue (bug_risk):** Narrow the scope of discovered host deletion to avoid removing unrelated hosts.
This currently deletes all discovered hosts whenever `provisioning_type == 'discovery'`, which can remove hosts belonging to other tests or users on the same Satellite. Please restrict the search (e.g., by org/location, hostname pattern, or run-specific metadata) so only resources created by this scenario are deleted.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+214
to
+216
| if provisioning_type == 'discovery': | ||
| for host in sat.api.DiscoveredHost().search(): | ||
| host.delete() |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Narrow the scope of discovered host deletion to avoid removing unrelated hosts.
This currently deletes all discovered hosts whenever provisioning_type == 'discovery', which can remove hosts belonging to other tests or users on the same Satellite. Please restrict the search (e.g., by org/location, hostname pattern, or run-specific metadata) so only resources created by this scenario are deleted.
* Fix Discovery provisioning tests * Apply SAT-33477 workaround to UI tests (cherry picked from commit 9ff92d0)
Gauravtalreja1
force-pushed
the
cherry-pick-6.18.z-9ff92d0ce1daf9b9ab0fed89f0861b2e1449e1aa
branch
from
March 30, 2026 10:57
01477e2 to
e05941d
Compare
Member
|
trigger: test-robottelo |
Collaborator
Author
|
PRT Result |
Satellite-QE
added
the
PRT-Failed
Collaborator
Author
|
PRT Result |
Member
|
Hey @archanaserver, this cherry-pick PR has a PRT failure. Could you take a look and let us know if it's ready to merge despite the failure? |
|
This pull request has not been updated in the past 45 days. |
|
This pull request is now being closed after stale warnings. |
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
4 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.
Cherrypick of PR: #20638
Problem Statement
Discovery provisioning tests are failing because
dhcpdis not restarted after building PXE templates in themodule_discovery_satfixture. Without the restart, dhcpd serves old cached configuration.Solution
Added
dhcpdservice restart in themodule_discovery_satfixture after all discovery configuration is complete. This ensures dhcpd picks up the updated PXE boot configuration and serves the correct discovery boot files to VMs.Related Issues
Fixes: SAT-39920
Summary by Sourcery
Adjust discovery provisioning tests and fixtures to better handle discovered host assignment and cleanup for discovery-based provisioning.
Bug Fixes:
Tests: