Skip to content

[6.18.z] Modifications in CLI and UI Libvirt CR to resolve key error - #20759

Closed
nacoool wants to merge 0 commit into
SatelliteQE:6.18.zfrom
nacoool:Test_Libvirt_6.18.z
Closed

[6.18.z] Modifications in CLI and UI Libvirt CR to resolve key error#20759
nacoool wants to merge 0 commit into
SatelliteQE:6.18.zfrom
nacoool:Test_Libvirt_6.18.z

Conversation

@nacoool

@nacoool nacoool commented Feb 10, 2026

Copy link
Copy Markdown
Contributor

Problem Statement

Cherry pick of Modifications in CLI and UI Libvirt CR to resolve key error failed

Solution

New PR to add CP manually for #20176

Related Issues

Summary by Sourcery

Update Libvirt provisioning tests and fixtures to improve reliability and handle missing status keys while adjusting PXE loaders and UI host lookup.

Bug Fixes:

  • Prevent key errors when reading host build status by safely accessing the status structure and enabling exception handling in the wait loop.
  • Skip UEFI and SecureBoot Libvirt provisioning scenarios when a known issue is open to avoid unstable failures.
  • Switch UI Libvirt end-to-end provisioning test to use the correct host search endpoint to retrieve host details without errors.

Enhancements:

  • Expand Libvirt end-to-end provisioning coverage to include BIOS PXE loader in addition to existing UEFI variants.
  • Improve IPv4 provisioning stability by clearing DHCP leases and restarting the DHCP service before running PXE-based workflows.

Tests:

  • Strengthen Libvirt CLI and UI end-to-end provisioning tests to be more resilient to environment issues and firmware-specific limitations.

@sourcery-ai

sourcery-ai Bot commented Feb 10, 2026

Copy link
Copy Markdown
Contributor
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates Libvirt compute resource provisioning tests (CLI and UI) to handle firmware-specific behavior, make host build-status checks more robust, and ensure IPv4 DHCP state is reset before provisioning, reducing flaky failures and key errors.

Sequence diagram for IPv4 Libvirt provisioning with DHCP reset

sequenceDiagram
    actor Tester
    participant PytestRunner
    participant Module_provisioning_sat
    participant Satellite
    participant DHCPServer

    Tester->>PytestRunner: run Libvirt compute resource tests
    PytestRunner->>Module_provisioning_sat: request Satellite provisioning fixture

    Module_provisioning_sat->>Satellite: create Satellite entities
    Module_provisioning_sat->>Satellite: create Libvirt compute resource

    alt network_type is IPv4
        Module_provisioning_sat->>Satellite: execute cat /dev/null > /var/lib/dhcpd/dhcpd.leases
        Satellite->>DHCPServer: clear dhcpd.leases file
        Module_provisioning_sat->>Satellite: execute systemctl restart dhcpd
        Satellite->>DHCPServer: restart dhcpd service
    else network_type is not IPv4
        Module_provisioning_sat-->>Satellite: skip DHCP reset
    end

    Module_provisioning_sat-->>PytestRunner: return sat, domain, subnet, provisioning_type
    PytestRunner-->>Tester: run CLI and UI Libvirt tests using provisioned Satellite
Loading

File-Level Changes

Change Details Files
Broaden PXE loader coverage and conditionally skip UEFI/SecureBoot when blocked by an open issue, while hardening the build-status wait logic in the CLI Libvirt end-to-end provisioning test.
  • Extend pxe_loader parameterization from only UEFI variants to include BIOS as well
  • Introduce an is_open('SAT-41340') check and skip the test when using UEFI or UEFI secure boot firmware to avoid unsupported scenarios
  • Change the wait_for predicate to safely access nested 'status' and 'build-status' keys using .get with defaults, avoiding key errors when status is missing
  • Enable exception handling in wait_for to tolerate transient CLI/lookup failures during host provisioning
tests/foreman/cli/test_computeresource_libvirt.py
Ensure clean DHCP state for IPv4-based provisioning by clearing leases and restarting dhcpd in the shared provisioning fixture.
  • Document that IPv4 provisioning now clears DHCP leases and restarts dhcpd to guarantee address assignment
  • After creating the provisioning host and related entities, conditionally clear /var/lib/dhcpd/dhcpd.leases and restart the dhcpd service when the Satellite network type is IPv4
pytest_fixtures/component/provision_pxe.py
Align the UI Libvirt end-to-end provisioning test with the updated host search API to avoid key errors and mismatches.
  • Switch from session.host.search to session.host_new.search when looking up the newly provisioned host
  • Store the search result in a variable and assert the Name field matches the expected FQDN
tests/foreman/ui/test_computeresource_libvirt.py

Possibly linked issues

  • #unknown: The PR is the manual cherry-pick implementing the Libvirt CLI/UI key error fix after auto-cherry-pick failed.
  • #[Failed-AutoCherryPick]: PR manually applies the Libvirt CLI/UI key error changes whose auto-cherry-pick failed in the issue.
  • #: The PR manually backports the Libvirt CR CLI/UI key-error fixes whose auto-cherry-pick failure is reported in the issue.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The skip condition for pxe_loader.vm_firmware checks ['uefi', 'uefi_secure_boot'] while the parametrized values are ['bios', 'uefi', 'secureboot']; verify that vm_firmware actually matches these strings (e.g., secureboot vs uefi_secure_boot) so the intended cases are reliably skipped.
  • The hardcoded dhcpd.leases path and systemctl restart dhcpd in module_provisioning_sat may be brittle across environments; consider centralizing these commands or adding a small helper that can handle alternative service names/paths or at least logs clearer errors when these commands fail.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The skip condition for `pxe_loader.vm_firmware` checks `['uefi', 'uefi_secure_boot']` while the parametrized values are `['bios', 'uefi', 'secureboot']`; verify that `vm_firmware` actually matches these strings (e.g., `secureboot` vs `uefi_secure_boot`) so the intended cases are reliably skipped.
- The hardcoded `dhcpd.leases` path and `systemctl restart dhcpd` in `module_provisioning_sat` may be brittle across environments; consider centralizing these commands or adding a small helper that can handle alternative service names/paths or at least logs clearer errors when these commands fail.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@nacoool

nacoool commented Feb 11, 2026

Copy link
Copy Markdown
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_computeresource_libvirt.py -k "test_positive_provision_end_to_end"

@nacoool

nacoool commented Feb 11, 2026

Copy link
Copy Markdown
Contributor Author
trigger: test-robottelo
pytest: tests/foreman/ui/test_computeresource_libvirt.py -k "test_positive_provision_end_to_end"
provisioning: true

@Gauravtalreja1 Gauravtalreja1 added TestFailure Issues and PRs related to a test failing in automation No-CherryPick PR doesnt need CherryPick to previous branches Failed_AutoCherryPick Issue for failed/conflicted auto cherry pick of PRs 6.18.z Introduced in or relating directly to Satellite 6.18 labels Feb 12, 2026
@nacoool nacoool changed the title cherrypick : Modifications in CLI and UI Libvirt CR to resolve key error [6.18.z] Modifications in CLI and UI Libvirt CR to resolve key error Feb 17, 2026
@nacoool nacoool closed this Feb 17, 2026
@nacoool
nacoool force-pushed the Test_Libvirt_6.18.z branch from 5f3bebf to 6f4c8db Compare February 17, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.18.z Introduced in or relating directly to Satellite 6.18 Failed_AutoCherryPick Issue for failed/conflicted auto cherry pick of PRs No-CherryPick PR doesnt need CherryPick to previous branches TestFailure Issues and PRs related to a test failing in automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants