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

feat: add data sources to extract node and device information #1042

Merged
merged 17 commits into from
Dec 28, 2023

Conversation

muresan
Copy link
Contributor

@muresan muresan commented Oct 24, 2023

Hello,

this PR adds support for new data sources:

  • libvirt_node_info - to retrieve node information and make it available to Terraform. (virsh nodeinfo)
  • libvirt_node_devices - retrieves the list of devices available in the node, can be filtered by capability. (virsh nodedev-list)
  • libvirt_node_device_info - retrieves information about a single device. (virsh nodedev-dumpxml)

Apart from the changes to docs and provider.go to plugin in the documentation and the new resources, changes are in new files.

Co-authored-by: Russell Bunch <doomslayer@hpe.com>
muresan and others added 2 commits November 27, 2023 13:05
Co-authored-by: Mike Beaumont <mjboamail@gmail.com>
Copy link
Collaborator

@michaelbeaumont michaelbeaumont left a comment

Choose a reason for hiding this comment

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

Thanks!

@michaelbeaumont michaelbeaumont changed the title Added support for data sources to extract node and device information feat: add data sources to extract node and device information Dec 28, 2023
@michaelbeaumont michaelbeaumont merged commit 7584609 into dmacvicar:main Dec 28, 2023
4 checks passed
@dmacvicar
Copy link
Owner

Still puzzled how the test ever passed:

=== RUN   TestAccLibvirtNodeInfoDataSource
    data_source_libvirt_node_info_test.go:11: Step 1/2 error: Check failed: Check 1/1 error: data.libvirt_node_info.info: Attribute 'cpus' didn't match "^\\d+", got ""
-```

d.Set("cpu_model", int8ToString(model))
d.Set("cpu_cores_total", cpus)
d.Set("cpu_cores_per_socket", cores)
d.Set("numa_nodes", nodes)
d.Set("cpu_sockets", sockets)
d.Set("cpu_threads_per_core", threads)
d.Set("numa_nodes", nodes)
d.Set("memory_total_kb", memory)
Test tests for an attribute cpu, which has never been set. It is clear integration tests were not run, and shows the need to get them to run in CI with nested virtualization.

memetb pushed a commit to memetb/terraform-provider-libvirt that referenced this pull request Sep 23, 2024
this is related to the conversation in issue dmacvicar#1074. The original implementation
of nodeinfo (as released in PR dmacvicar#1042) has a bug - which may very well be easily
fixed - however the current PR and branch specifically addressed that issue by
calling virConnectGetCapabilities instead of virNodeGetInfo. Instead of doing
this, a new feature "host_capabilities" is added, and the nodeinfo code is left
unchanged (it may be addressed in a separate PR)
dmacvicar pushed a commit to jimnydev/terraform-provider-libvirt that referenced this pull request Sep 28, 2024
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