Skip to content

During install tests, avoid VM being shutdown before PDB can run - #563

Open
ydirson wants to merge 2 commits into
masterfrom
install/avoid-setup-failure
Open

During install tests, avoid VM being shutdown before PDB can run#563
ydirson wants to merge 2 commits into
masterfrom
install/avoid-setup-failure

Conversation

@ydirson

@ydirson ydirson commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

When booting a nested host with an installer ISO, in case of failure to get to the state where we can read it using ssh, we could not inspect the host once --pdb stopped the test, because the exception was firing from fixture setup code.

ydirson added 2 commits June 1, 2026 18:18
Makes the management of the temporary ISO self-contained.

Signed-off-by: Yann Dirson <yann.dirson@vates.tech>
vm_booted_with_installer fixture would shutdown an host under installation
if it fails to boot far enough to be driven by the test (in the fixture
setup stage).

This changes the exception handling to record the failure, make sure the
calling test or fixture must fail, and let the cleanup stage do the
shutdown when needed.

This changes the fixture signature, which cannot be detected today by
type checkers, so the fixture name is changed as well to help devs catch
code in pending PRs.

Signed-off-by: Yann Dirson <yann.dirson@vates.tech>
@ydirson
ydirson requested a review from a team as a code owner June 2, 2026 09:24
@glehmann

glehmann commented Jun 2, 2026

Copy link
Copy Markdown
Member

As discussed earlier, it would be nice if we could avoid yielding None :)

@stormi stormi changed the title During install tests, avoid VM behing shutdown before PDB can run During install tests, avoid VM being shutdown before PDB can run Jun 8, 2026
@stormi

stormi commented Jun 19, 2026

Copy link
Copy Markdown
Member

As discussed earlier, it would be nice if we could avoid yielding None :)

Is it a request for change, or just a suggestion but then you didn't approve for some reason?

@glehmann
glehmann requested review from glehmann and vxgmichel August 3, 2026 14:29
@stormi
stormi removed the request for review from a team August 4, 2026 11:13

@vxgmichel vxgmichel 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.

I think this change is very useful, but I agree with @glehmann that it might done in a way that is more straight forward.

In particular, having (try_)booting_vm_installer no longer failing when it could not boot gives the wrong impression that this might be expected for some specific tests. As far as I understand this is not the case, but a way to address the valid issue of being able to drop in a PDB before the cleanup is performed when the booting fails.

Instead, we might want to add a defer/request.addFinalizer to booting_vm_installer, i.e remove the try-except-finally statements and use something like this instead:

    def cleanup():
        if host_vm is not None:
            host_vm.shutdown(force=True)
        if remote_iso:
            host.pool.remove_iso(remote_iso)
    defer(cleanup)

This should achieve the expected result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants