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

Implement --check support for the libvirt.virt module #183

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

lowjoel
Copy link

@lowjoel lowjoel commented Dec 1, 2024

SUMMARY

Implement --check support for the libvirt.virt module

Fixes #98.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

virt

ADDITIONAL INFORMATION

Initial implementation to support --check mode.

This adds checking to the Virt class to make check mode calls no-ops but returning valid information where possible. For the get methods this doesn't affect behaviour, but for state changes and for defining domains this will make the Virt class do nothing in check mode.

For defining a new domain, this will pretty-print both XML definitions to make the diff output clean. It's not perfect because libvirt adds additional attributes when defining a domain that are implicit. Those will not change the domain if they are not specified in the input XML.

TASK [libvirt-domain : Define Domain (domain)] ***********************************************************************************************************************************************************************************************************************************************
--- before
+++ after
@@ -1,37 +1,37 @@
-<domain type="kvm">
+<domain xmlns:qemu="http://libvirt.org/schemas/domain/qemu/1.0" type="kvm">
   <name>domain</name>
   <uuid>snip</uuid>
-  <title>Windows 11</title>
-  <description>Windows 11 21H2</description>
+  <title>Windows 10</title>
+  <description>Windows 10 21H2</description>
   <metadata>
     <libosinfo:libosinfo xmlns:libosinfo="http://libosinfo.org/xmlns/libvirt/domain/1.0">
...snip

@Andersson007
Copy link
Contributor

@lowjoel hello, thanks for your effort! please also add a changelog fragment of minor_changes type https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment

@lowjoel
Copy link
Author

lowjoel commented Dec 2, 2024

Thanks! I'm not entirely sure this is ready to merge yet; I think the state changes aren't reported as changes (not sure). The state changes should be reported as changes but the messages are likely meaningless - not sure what should be best. But I've added the changelog fragment (hope it's correct)

@csmart
Copy link
Collaborator

csmart commented Dec 3, 2024

hi @lowjoel thanks for this contribution! Wondering whether it's possible to add some tests to validate the functionality, like is done with virt_pool? I think it'd be great to make sure it's behaving as expected. Cheers!

@lowjoel
Copy link
Author

lowjoel commented Dec 3, 2024

Pretty much duplicated it and replaced it with domain, is there some CI infrastructure I can test that it all works? I could figure out how to test this out using Docker or the like but it'll need some rummaging through docs and could take a while.

@Andersson007
Copy link
Contributor

Pretty much duplicated it and replaced it with domain, is there some CI infrastructure I can test that it all works? I could figure out how to test this out using Docker or the like but it'll need some rummaging through docs and could take a while.

@lowjoel hello, please take a look at

@lowjoel
Copy link
Author

lowjoel commented Dec 10, 2024

@lowjoel hello, please take a look at

thanks! though in my case the harder part is to have a libvirt instance to test with because I (obviously) shouldn't be testing this in my production setup.

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.

ansible-playbook --check mode/diff support for virt module
3 participants