smutel / terraform-provider-netbox

Terraform provider for Netbox
ISC License
58 stars 19 forks source link

feat: Add function to modify API requests #150

Closed amhn closed 1 year ago

amhn commented 1 year ago

I looked into the behaviour of go-netbox a little more. Turns out it is possible to hook into the request before it is sent.

This let's us do two things:

I did only implement it for netbox_virtualization_vms yet. Let me know what you think of this approach. This would help for the primary_ip6 stuff, at the very least avoiding the forced renewal of IPs.

Fix #151 Fix #152

smutel commented 1 year ago

Hello,

I don't test your branch but it could be a good idea.

I am currently facing an issue with primary ip. When I change primary_ip4 field, I need to set the primary_ip6 with his older value. This does not work with concurrent calls on the same vm interface.

smutel commented 1 year ago

I will try to test it tomorrow

amhn commented 1 year ago

With this branch it is not needed to populate fields other than the ones that have changed.

There is still a concurrency issue in netbox which is triggered by changing primary_ip4 and primary_ip6 in parallell. I have seen that in the terraform_vault provider they work around this with mutexes per api endpoint. Perhaps this is worth looking at.

smutel commented 1 year ago

When I remove the CPU, the primary interface is removed also. With a tcpdump I saw the request args below:

{"primary_ip4":null,"primary_ip6":null,"vcpus":null}
smutel commented 1 year ago

I think it's due to https://github.com/smutel/go-netbox/pull/42. Perhaps it's better to revert it and to move forward with this PR ?

smutel commented 1 year ago

I tested it without https://github.com/smutel/go-netbox/pull/42 and it is working fine.
It will definitively help to add primary_ip field.