dmacvicar / terraform-provider-libvirt

Terraform provider to provision infrastructure with Linux's KVM using libvirt
Apache License 2.0
1.59k stars 458 forks source link

RFC: Force new resource when we can't update an attribute due to libvirt API limitations (idempotency) #787

Closed MalloZup closed 3 years ago

MalloZup commented 4 years ago

Libvirt API can't update all attributes for a specific Resource.

An example I have in mind is for the network resource where one can't update all attributes.

See for example this issue: https://github.com/dmacvicar/terraform-provider-libvirt/issues/784#issuecomment-699527570

This is quite ugly from UX perspective in terraform. As USER, i would expect that when I change a Network resource like domain in this case

  # libvirt_network.app-real2 will be updated in-place
  ~ resource "libvirt_network" "app-real2" {
        autostart = true
        bridge    = "virbr3"
      ~ domain    = "foo" -> "bro"

I would expect that the change will be applied. Right now, either we fail in some circumstances, or we just say OK, but in reality is not changed.

I suppose also that Storage/pool or Domains cannot update some attributes, because libvirt cannot update them technically for some X limitations.

In this context, I think a valid approach would be to to use ForceNew: true attribute. This will make idempotent our calls.

Unfortunately, there was no structure on how we set the ForceNew: true or false historically on the variables.. It is quite messy in this regard. Some variable has it, some not without much structure.

I think that should be our guideline, when Libvirt cannot update a Value of resource, this should ForceNew: true because it is the only way to make the whole provider to work in a immutable infrastructure/idempotent way, on all CRUD operations. If we can't support the Update operation(because libvirt limitations) we should for new resources.

Opinions on this? Alternatives?

MalloZup commented 3 years ago

we should document and fix some minor issues as discussed in https://github.com/dmacvicar/terraform-provider-libvirt/pull/788 last comments