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

Repair virt_query outputter #655

Merged
merged 3 commits into from
Feb 21, 2025

Conversation

tacerus
Copy link
Member

@tacerus tacerus commented May 30, 2024

Upstream PR

saltstack/salt#65843

What does this PR do?

What issues does this PR fix or reference?

Fixes: saltstack/salt#65841

Previous Behavior

Outputter returns traceback.

New Behavior

Outputter returns pretty output:

# salt-run --out=virt_query virt.query host=squanchy.infra.opensuse.org
squanchy.infra.opensuse.org
  thor1.infra.opensuse.org
    CPU: 1
    Memory: 1000000
    State: running
    Disk - vdb:
      Size: 2541887488
      File: /data0/kvm/disks/thor1.infra.opensuse.org_root.qcow2
      File Format: qcow2
    NIC - fe:cd:47:19:e7:86:
      Source: x-os-thor
      Type: direct
# more VMs ...

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

^ I'm not sure about tests for this, some other outputters have tests in tests/pytests/unit/output/, but I'm not sure it's feasible for this one, it would need to hook into the virt module tests somehow to generate a VM fixture?

Commits signed with GPG?

Yes

tacerus and others added 3 commits May 31, 2024 01:11
Existing code was not functional.
Only return if a dictionary is passed and reference the correct
data.

Signed-off-by: Georg Pfuetzenreuter <[email protected]>
(cherry picked from commit e3c365ad8f385121aa878950e13892d986d79656)
Disk data in Libvirt VMs does not contain a disk size if the disk
references a block device.
Skip the field for such disks instead of failing with a key error.

Signed-off-by: Georg Pfuetzenreuter <[email protected]>
(cherry picked from commit ed73abd44117ad745e9c91f2b33caf04402b117c)
@meaksh
Copy link
Member

meaksh commented Feb 21, 2025

Thanks for this contribution @tacerus.

I just added here a unit test file for virt_query so we actually test this. You probably want to include it to the upstream PR.

@meaksh meaksh merged commit 3255067 into openSUSE:openSUSE/release/3006.0 Feb 21, 2025
8 checks passed
@tacerus tacerus deleted the virt_query-opensuse branch February 21, 2025 17:10
@tacerus
Copy link
Member Author

tacerus commented Feb 21, 2025

Thanks for adding a test @meaksh, I'll suggest it upstream.

@tacerus tacerus restored the virt_query-opensuse branch February 21, 2025 17:35
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.

[BUG] virt_query outputter returns traceback
2 participants