feat(vm): implement filtering in vms data source.

[konstantin-kornienko]

• Additional attributes for vm data source (status, template)

Contributor's Note

• I have added / updated documentation in /docs for any user-facing features or additions.
• I have added / updated acceptance tests in /fwprovider/tests for any new or updated resources / data sources.
• I have ran make example to verify that the change works as expected.

Proof of Work

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates #1010

[konstantin-kornienko]

Trying to implement filtering in vms data source.

I'm not satisfied with the result, because:

• With new filter block approach it makes sense to move node_name and tags arguments to the corresponding blocks. But it'd be a breaking change. Like:

data "proxmox_virtual_environment_vms" "ubuntu_templates" {
  filter {
     name = "tags"
     values = ["template", "latest"]  # also logic here will be AND, instead of OR in the PR
  }

  filter {
    name   = "node_name"
    values = ["^proxmox-node-(1|2|3)$"]
  }
}

2. Currently the vms data source is searching for VMs using api.Node(nodeName).VM(0).ListVMs call. But looks like /cluster/resources can be used here. It's super fast and can return information about all VMs on all nodes with all required properties for filtering. But maybe it has some disadvantages I'm not aware of.

[vanillaSprinkles]

is the values always in regex mode? i think having 2x modes would be better; such as an exact string match along the lines of values = "exact_string", and regex would be wrapped in /, like values = "/^regex/" ( similar to https://developer.hashicorp.com/terraform/language/functions/replace )

in regards to /cluster/resources; yes i think it should be fine (not sure if proxmox 7 and below has the same api calls) - worst case scenario for supporting both would be choosing a path based on pvesh get /version and handling both api-paths

[bpg]

Is the values always in regex mode? I think having 2x modes would be better, such as an exact string match along the lines of values = "exact_string", and regex would be wrapped in /, like values = "/^regex/" (similar to https://developer.hashicorp.com/terraform/language/functions/replace).

I think regex only is fine, if this behavior is clearly documented. One can always use ^exact_string$ match if needed (and we can document that as well), but I believe in most use cases the default regex matching will be fine.

In regards to /cluster/resources; yes, I think it should be fine (not sure if Proxmox 7 and below has the same API calls) - worst case scenario for supporting both would be choosing a path based on pvesh get /version and handling both API paths.

PVE 7.x is not officially supported by the provider, and I think some features are not working in 7.x anymore (like creating a VM disk from an image). I don't really like to put extra effort into maintaining 7.x compatibility, to be honest.

[bpg]

Trying to implement filtering in vms data source.

I'm not satisfied with the result, because:

• With new filter block approach it makes sense to move node_name and tags arguments to the corresponding blocks. But it'd be a breaking change. Like:

data "proxmox_virtual_environment_vms" "ubuntu_templates" {
  filter {
     name = "tags"
     values = ["template", "latest"]  # also logic here will be AND, instead of OR in the PR
  }

  filter {
    name   = "node_name"
    values = ["^proxmox-node-(1|2|3)$"]
  }
}

I would say, mark the top level node_name as deprecated, and add a note to use filter instead. We'll eventually clean this up while re-implementing the datasource using Plugin Framework (after #1231)

2. Currently the vms data source is searching for VMs using api.Node(nodeName).VM(0).ListVMs call. But looks like /cluster/resources can be used here. It's super fast and can return information about all VMs on all nodes with all required properties for filtering. But maybe it has some disadvantages I'm not aware of.

Go for it! 🙂 I probably just didn't think about the cluster API when implementing this datasource.

[konstantin-kornienko]

Okay! Trying to implement all suggestions.

1. Just verified that /cluster/resources works fine in Proxmox v7.
   But... it's not an option, sorry. It provides all required information except... tags (((

2. Regarding regex. @vanillaSprinkles what do you think about this format?

data "proxmox_virtual_environment_vms" "template_example" {
  filter {
    name   = "status"
    values = ["stopped"]
   // regex = false by default. Full string match in this case
  }

  filter {
    name   = "name"
    regex  = true
    values = [".*ubuntu.*"]
  }
}

[bpg]

@konstantin-kornienko Thanks a lot for putting this together! It's a great improvement for the datasource!

LGTM! 🚀

[bpg]

@all-contributors please add @konstantin-kornienko for code, ideas

[allcontributors]

@bpg

I've put up a pull request to add @konstantin-kornienko! 🎉