search

e-breuninger / terraform-provider-netbox

Terraform provider to interact with Netbox
https://registry.terraform.io/providers/e-breuninger/netbox/latest/docs
Mozilla Public License 2.0
178 stars 130 forks source link

Bug fix: do not update `primary_ipv(4|6)` values on `netbox_virtual_machine` #609

Open ad8lmondy opened 3 months ago

ad8lmondy commented 3 months ago

TL;DR: the primary_ipv4 and primary_ipv6 properties on the netbox_virtual_machine resource create an implicit loop, since these values are detemined by making a netbox_primary_ip resource (which itself must explicitly depend on the netbox_virtual_machine). This can cause Terraform plans that need two applies to work, with the first erroring out. The fix is to remove the updates to the primaryIP(4|6)Values.

Longer version: Creating a netbox_primary_ip resource implicitly modifies the related netbox_virtual_machine, since the latter keeps a record of the primaryIP4Value and primaryIP6Value - but this only happens when resourceNetboxVirtualMachineRead is called, so this info lags behind.

However, it normally works fine, but when the netbox_virtual_machine resource is modified, and the netbox_primary_ip is deleted, it leads to an issue where when NetBox tries to update the VM, it can no longer find the IP address ID.

This is because netbox_virtual_machine doesn't know it has an implicit dependency on the netbox_primary_ip, while at the same time, netbox_primary_ip has an explicit dependency on the netbox_virtual_machine resource (note that this is a loop). Thus, Terraform reads the state of netbox_virtual_machine, including the primary IPs, then deletes the netbox_primary_ip resource, and then (due to the explicit dependency) tries to modify the netbox_virtual_machine resource with now out-of-date info about the primaryIP(4|6)Values.

The solution is to remove updating the primaryIP(4|6)Values when updating a VM. If a netbox_primary_ip resource is defined, the relationship is still preserved - but as before this fix, the info lags behind, since it's only updated on a resourceNetboxVirtualMachineRead. I don't think we can do much about that for now, but this at least removes an actual error that can be hit.

Two tests were added in regards to this issue:

They are a little odd, in that they have repeated steps, which is there to force a call to resourceNetboxVirtualMachineRead. When the code in netbox/resource_netbox_virtual_machine.go still has the calls to update primaryIP(4|6)Values in resourceNetboxVirtualMachineUpdate, the remove_primaryIPAddress test fails. The changePrimaryIPAddress was put in just to ensure the values still could be retrieved after the code was removed.

ad8lmondy commented 3 months ago

Seems related to this comment too: https://github.com/e-breuninger/terraform-provider-netbox/issues/538#issuecomment-2098842947

ad8lmondy commented 3 months ago

hmm, this isn't working quite as I hoped - I'm starting to loose primary IP on some of my VMs as I perform other operations.

I've 'fixed' it by adding a replace_triggered_by block to the netbox_primary_ip resource:

resource "netbox_primary_ip" "test" {
  ip_address_id      = resource.netbox_available_ip_address.ip_addr.id
  virtual_machine_id = resource.netbox_virtual_machine.vm.id
  lifecycle {
    replace_triggered_by = [
      netbox_virtual_machine.vm
    ]
  }

Which seems to work - though it's much harder when these things are being set in for_each blocks, for example.

I've had a look at the code in netbox/resource_netbox_primary_ip.go and netbox/resource_netbox_virtual_machine.go, and I can't really see how the netbox_virtual_machine update code would be able to find a netbox_primary_ip assosciated with the VM, without setting up an explicit cycle.

Or another way to put it: having the separate resource of netbox_primary_ip seems not very compatible with Terraform's model of things..? But I could easily be missing something!

A possible solution could be to allow the resource netbox_virtual_machine (or netbox_device, which I just remembered about :( ) to set the primary_ip value to the relevant netbox_ip_address id - though cycles might come back in doing that...

Any ideas or input welcome :)

fbreckle commented 3 months ago

As for your last question: Setting the primary ip directly in the VM does definitely not work, because it will create a loop. See here