vatesfr / terraform-provider-xenorchestra

Xen Orchestra provider for Terraform
MIT License
155 stars 32 forks source link

Consider allowing changes to VM resource's `cloud_config` argument to force recreation #205

Open ddelnano opened 2 years ago

ddelnano commented 2 years ago

This was discussed in Discord.

It is surprising that the cloud_config argument is capable of being changed on a xenorchestra_vm when it likely will have no effect. The aws_instance resource in the AWS terraform provider allows a similar configuration through launch templates. This argument causes the aws_instance resource to be recreated and so we should consider introducing this breaking change to fix this behavior and align it with how other providers treat cloud config / boot time configuration.

Screenshot_20220718_224909

4censord commented 2 years ago

That is already possible with the lifecycle meta argument: using something like:

resource "xenorchestra_cloud_config" "user_data" { }
resource "xenorchestra_cloud_config" "network_data" { }

resource "xenorchestra_vm" "vm" {
  lifecycle {
    replace_triggered_by = [
      xenorchestra_cloud_config.user_data,
      xenorchestra_cloud_config.network_data,
    ]
  }
}

Terraform replaces the vm whenever user_data or network_data has any changes.

That looks something like this

# xenorchestra_vm.vm will be replaced due to changes in replace_triggered_by
-/+ resource "xenorchestra_vm" "vm" {
      - blocked_operations   = [] -> null
      ~ cloud_config         = {REDACTED}
      ~ id                   = {REDACTED} -> (known after apply)
      ~ ipv4_addresses       = [
          - {REDACTED},
        ] -> (known after apply)
      ~ ipv6_addresses       = [
          - {REDACTED},
        ] -> (known after apply)
      ~ power_state          = "Running" -> (known after apply)
        tags                 = [
            {REDACTED}
        ]
        # (16 unchanged attributes hidden)
      ~ cdrom {
          ~ id = {REDACTED} -> {REDACTED}
        }
      ~ disk {
          - attached   = true -> null
          ~ position   = "0" -> (known after apply)
          ~ vbd_id     = {REDACTED} -> (known after apply)
          ~ vdi_id     = {REDACTED} -> (known after apply)
            # (3 unchanged attributes hidden)
        }
      ~ network {
          - attached       = true -> null
          ~ device         = "0" -> (known after apply)
          ~ ipv4_addresses = [
              - {REDACTED},
            ] -> (known after apply)
          ~ ipv6_addresses = [
              - {REDACTED},
            ] -> (known after apply)
          ~ mac_address    = {REDACTED} -> (known after apply)
            # (1 unchanged attribute hidden)
        }
    }

ddelnano commented 2 years ago

@4censord thanks for sharing that workaround. I still think it makes sense to unify this against what other similar providers are doing, but it's great that Terraform has this flexibility.

4censord commented 2 years ago

I would also like to have the option for a change on the cloud-init data to force a power cycle of the vm. This does not seem possible with lifecycle configuration only.

ddelnano commented 2 years ago

to force a power cycle of the vm.

@4censord what I described in the issue description would be to cause the VM to be destroyed and recreated. Are you saying that if your case you want the VM to be rebooted?

4censord commented 2 years ago

Yes, I would like to have the option to only reboot the vm I propose to do this via an option that allows the user to choose:

resource "xenorchestra_vm" "vm" {
  on_cloudinit_change = "reboot" # stop the vm, do the change, start the vm
  on_cloudinit_change = "recreate" # destroy and recreate with the change
  on_cloudinit_change = "ignore" # just change it (current behavior, defaul)
  on_cloudinit_change = "fail" # Do not allow changes to the cloudinit config
}