information returned by node-info from recent PR #1042 is incorrect

System Information

Linux distribution

any

Terraform version

terraform -v
Terraform v1.5.7
on linux_amd64

Provider and libvirt versions

terraform-provider-libvirt -version

Description of Issue/Question

Setup

I recently implemented a very similar feature in PR #1073 without realizing that #1042 had very recently been merged. I'm happy to close my PR however the output of the node info is incorrect and less expansive than it could.

Example:

// main.tf
terraform {
  required_version = ">= 0.13"
  required_providers {
    libvirt = {
      #      source  = "dmacvicar/libvirt"
      source = "terraform.local/local/libvirt"
      version = "1.0.0"
    }
  }
}

provider "libvirt" {
  uri = "qemu+ssh://${var.target}/system?sshauth=privkey&no_verify"
}

data "libvirt_node_info" "host" {
}

output node_info {
  value = data.libvirt_node_info.host
}

produces

Changes to Outputs:
  + node_info = {
      + cpu_cores_per_socket = 2
      + cpu_cores_total      = 2
      + cpu_model            = "aarch64"  # Incorrect
      + cpu_sockets          = 1
      + cpu_threads_per_core = 1
      + id                   = "1052227631"
      + memory_total_kb      = 2229728
      + numa_nodes           = 1
    }

By contrast, this is the output of the node info as implemented in PR #1073

Changes to Outputs:
  + node_info = {
      + arch                      = "aarch64"
      + cores                     = 2
      + dies                      = 1
      + features                  = [
          + "fp",
          + "asimd",
          + "evtstrm",
          + "aes",
          + "pmull",
          + "sha1",
          + "sha2",
          + "crc32",
          + "cpuid",
        ]
      + id                        = "d4e1bf70-6374-45b7-a970-fb584a1b4062" # this is the underlying libvirt native id
      + live_migration_support    = true
      + live_migration_transports = [
          + "tcp",
          + "rdma",
        ]
      + model                     = "cortex-a72"  # missing in PR 1042
      + sockets                   = 1
      + threads                   = 1
      + topology                  = [] # not yet implemented but can be expanded on
      + vendor                    = "ARM"  # missing in PR 1042
    }

The implementation for #1073 libvirt/data_source_libvirt_nodeinfo.go pulls and parses the direct XML dump from virt.

I believe it to be more expressive and open to upgrading, especially given that the actual topology can be decoded with a bit extra effort.

I am working on two other PR's right now (#1072 and #1059). I'd be happy to rework #1073 to fix the above. My only ask would be to name the parameters as I've done (i.e. cores not cpu_cores_per_socket) mainly because I've used the sames names as libvirt capabilities schema

@michaelbeaumont @rustydb @dmacvicar

Hi,

at the time I implemented node-info using https://libvirt.org/html/libvirt-libvirt-host.html#virNodeInfo which returns what you saw (with incorrect data) and I did not see that virConnectGetCapabilities exists. Since a release was not cut that includes this format, I'd say feel free to update it to the capabilities structure.

Alternatively, you can add a libvirt_node_capabilities data source, which would map to the virConnectGetCapabilities better, if someone is already using the current node_info (doubt it).

Edit: what I needed at the time was a way to determine total memory on the host and cpu cores, in order to make decisions about the number of cpu cores and memory allocated to VMs. On second thought I would recommend implementing an additional data source named libvirt_node_capabilities because memory doesn't "fit" in the capabilities object and to better map 1:1 what libvirt has with what the provider providess

dmacvicar / terraform-provider-libvirt