From 1dbf18f1548d0a6f2383c6472dfcf3a11a01ecc6 Mon Sep 17 00:00:00 2001 From: Thibault Bustarret Date: Thu, 27 Jun 2024 17:29:21 +0200 Subject: [PATCH] fix primary ip race condition on update --- netbox/resource_netbox_device.go | 12 - .../resource_netbox_device_primary_ip_test.go | 268 ++++++++++++++++++ 2 files changed, 268 insertions(+), 12 deletions(-) diff --git a/netbox/resource_netbox_device.go b/netbox/resource_netbox_device.go index 661b744e..6e854c04 100644 --- a/netbox/resource_netbox_device.go +++ b/netbox/resource_netbox_device.go @@ -464,18 +464,6 @@ func resourceNetboxDeviceUpdate(ctx context.Context, d *schema.ResourceData, m i data.ConfigTemplate = &configTemplateID } - primaryIP4Value, ok := d.GetOk("primary_ipv4") - if ok { - primaryIP4 := int64(primaryIP4Value.(int)) - data.PrimaryIp4 = &primaryIP4 - } - - primaryIP6Value, ok := d.GetOk("primary_ipv6") - if ok { - primaryIP6 := int64(primaryIP6Value.(int)) - data.PrimaryIp6 = &primaryIP6 - } - data.Rack = getOptionalInt(d, "rack_id") data.Face = getOptionalStr(d, "rack_face", false) data.Position = getOptionalFloat(d, "rack_position") diff --git a/netbox/resource_netbox_device_primary_ip_test.go b/netbox/resource_netbox_device_primary_ip_test.go index d681fc85..da9d78b3 100644 --- a/netbox/resource_netbox_device_primary_ip_test.go +++ b/netbox/resource_netbox_device_primary_ip_test.go @@ -172,3 +172,271 @@ resource "netbox_device_primary_ip" "test_v6" { }, }) } + +func TestAccNetboxDevicePrimaryIP4_removePrimary(t *testing.T) { + testSlug := "pr_ip_removePrimary" + testName := testAccGetTestName(testSlug) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccNetboxDevicePrimaryIPFullDependencies(testName) + fmt.Sprintf(` +resource "netbox_device" "test2" { + name = "%[1]s" + role_id = netbox_device_role.test.id + site_id = netbox_site.test.id + device_type_id = netbox_device_type.test.id + cluster_id = netbox_cluster.test.id + platform_id = netbox_platform.test.id + location_id = netbox_location.test.id + comments = "thisisacomment" + status = "planned" + rack_id = netbox_rack.test.id + rack_face = "front" + rack_position = 11 + + tags = [netbox_tag.test.name] +} + +resource "netbox_device_interface" "test2" { + device_id = netbox_device.test2.id + name = "%[1]s" + type = "1000base-t" +} + +resource "netbox_ip_address" "test_v4_2" { + ip_address = "1.1.1.16/32" + status = "active" + interface_id = netbox_device_interface.test2.id + object_type = "dcim.interface" +} + +resource "netbox_device_primary_ip" "test_v4_2" { + device_id = netbox_device.test2.id + ip_address_id = netbox_ip_address.test_v4_2.id +}`, testName), + }, + // A repeated second step is required, so that the resource "netbox_device" "test2" goes through a resourceNetboxDeviceRead cycle + // This is needed because adding a netbox_device_primary_ip updates the netbox_device + { + Config: testAccNetboxDevicePrimaryIPFullDependencies(testName) + fmt.Sprintf(` +resource "netbox_device" "test2" { + name = "%[1]s" + role_id = netbox_device_role.test.id + site_id = netbox_site.test.id + device_type_id = netbox_device_type.test.id + cluster_id = netbox_cluster.test.id + platform_id = netbox_platform.test.id + location_id = netbox_location.test.id + comments = "thisisacomment" + status = "planned" + rack_id = netbox_rack.test.id + rack_face = "front" + rack_position = 11 + + tags = [netbox_tag.test.name] +} + +resource "netbox_device_interface" "test2" { + device_id = netbox_device.test2.id + name = "%[1]s" + type = "1000base-t" +} + +resource "netbox_ip_address" "test_v4_2" { + ip_address = "1.1.1.16/32" + status = "active" + interface_id = netbox_device_interface.test2.id + object_type = "dcim.interface" +} + +resource "netbox_device_primary_ip" "test_v4_2" { + device_id = netbox_device.test2.id + ip_address_id = netbox_ip_address.test_v4_2.id +}`, testName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrPair("netbox_device_primary_ip.test_v4_2", "device_id", "netbox_device.test2", "id"), + resource.TestCheckResourceAttrPair("netbox_device_primary_ip.test_v4_2", "ip_address_id", "netbox_ip_address.test_v4_2", "id"), + ), + }, + // Now we do 2 things: modify netbox_device.test2 (changing the comment value), AND we remove the IP and primary IP + // This fails with: + // Error: [PUT /dcim/devices/{id}/][400] dcim_devices_update default {"primary_ip4":["Related object not found using the provided numeric ID: 14"]} + // because (I think) that the device is doing 1) a read of the current state, 2) the deletion of the primary IP then modifies the device, 3) the device then tries to write its changes, but its now out of date + { + Config: testAccNetboxDevicePrimaryIPFullDependencies(testName) + fmt.Sprintf(` +resource "netbox_device" "test2" { + name = "%[1]s" + role_id = netbox_device_role.test.id + site_id = netbox_site.test.id + device_type_id = netbox_device_type.test.id + cluster_id = netbox_cluster.test.id + platform_id = netbox_platform.test.id + location_id = netbox_location.test.id + comments = "thisisacomment with changes" + status = "planned" + rack_id = netbox_rack.test.id + rack_face = "front" + rack_position = 11 + + tags = [netbox_tag.test.name] +} + +resource "netbox_device_interface" "test2" { + device_id = netbox_device.test2.id + name = "%[1]s" + type = "1000base-t" +} +`, testName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("netbox_device.test2", "name", testName), + resource.TestCheckResourceAttr("netbox_device.test2", "primary_ipv4", "0"), + resource.TestCheckResourceAttr("netbox_device.test2", "tags.#", "1"), + resource.TestCheckResourceAttr("netbox_device.test2", "tags.0", testName), + resource.TestCheckResourceAttr("netbox_device.test2", "status", "planned"), + ), + }, + }, + }) +} + +func TestAccNetboxDevicePrimaryIP4_updateDevice(t *testing.T) { + testSlug := "pr_ip_updateDevice" + testName := testAccGetTestName(testSlug) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccNetboxDevicePrimaryIPFullDependencies(testName) + fmt.Sprintf(` +resource "netbox_device" "test3" { + name = "%[1]s" + role_id = netbox_device_role.test.id + site_id = netbox_site.test.id + device_type_id = netbox_device_type.test.id + cluster_id = netbox_cluster.test.id + platform_id = netbox_platform.test.id + location_id = netbox_location.test.id + comments = "comment1" + status = "planned" + rack_id = netbox_rack.test.id + rack_face = "front" + rack_position = 11 + + tags = [netbox_tag.test.name] +} + +resource "netbox_device_interface" "test3" { + device_id = netbox_device.test3.id + name = "%[1]s" + type = "1000base-t" +} + +resource "netbox_ip_address" "test_v4_3" { + ip_address = "1.1.1.18/32" + status = "active" + interface_id = netbox_device_interface.test3.id + object_type = "dcim.interface" +} + +resource "netbox_device_primary_ip" "test_v4_3" { + device_id = netbox_device.test3.id + ip_address_id = netbox_ip_address.test_v4_3.id +}`, testName), + }, + // A repeated second step is required, so that the resource "netbox_device" "test2" goes through a resourceNetboxDeviceRead cycle + // This is needed because adding a netbox_device_primary_ip updates the netbox_device + { + Config: testAccNetboxDevicePrimaryIPFullDependencies(testName) + fmt.Sprintf(` +resource "netbox_device" "test3" { + name = "%[1]s" + role_id = netbox_device_role.test.id + site_id = netbox_site.test.id + device_type_id = netbox_device_type.test.id + cluster_id = netbox_cluster.test.id + platform_id = netbox_platform.test.id + location_id = netbox_location.test.id + comments = "comment1" + status = "planned" + rack_id = netbox_rack.test.id + rack_face = "front" + rack_position = 11 + + tags = [netbox_tag.test.name] +} + +resource "netbox_device_interface" "test3" { + device_id = netbox_device.test3.id + name = "%[1]s" + type = "1000base-t" +} + +resource "netbox_ip_address" "test_v4_3" { + ip_address = "1.1.1.18/32" + status = "active" + interface_id = netbox_device_interface.test3.id + object_type = "dcim.interface" +} + +resource "netbox_device_primary_ip" "test_v4_3" { + device_id = netbox_device.test3.id + ip_address_id = netbox_ip_address.test_v4_3.id +}`, testName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrPair("netbox_device_primary_ip.test_v4_3", "device_id", "netbox_device.test3", "id"), + resource.TestCheckResourceAttrPair("netbox_device_primary_ip.test_v4_3", "ip_address_id", "netbox_ip_address.test_v4_3", "id"), + ), + }, + // Now we do 2 things: modify netbox_device.test3 (changing the comment value), AND we remove the IP and primary IP + // This fails with: + // Error: [PUT /dcim/devices/{id}/][400] dcim_devices_update default {"primary_ip4":["Related object not found using the provided numeric ID: 14"]} + // because (I think) that the device is doing 1) a read of the current state, 2) the deletion of the primary IP then modifies the device, 3) the device then tries to write its changes, but its now out of date + { + Config: testAccNetboxDevicePrimaryIPFullDependencies(testName) + fmt.Sprintf(` +resource "netbox_device" "test3" { + name = "%[1]s" + role_id = netbox_device_role.test.id + site_id = netbox_site.test.id + device_type_id = netbox_device_type.test.id + cluster_id = netbox_cluster.test.id + platform_id = netbox_platform.test.id + location_id = netbox_location.test.id + comments = "comment2" + status = "planned" + rack_id = netbox_rack.test.id + rack_face = "front" + rack_position = 11 + + tags = [netbox_tag.test.name] +} + +resource "netbox_device_interface" "test3" { + device_id = netbox_device.test3.id + name = "%[1]s" + type = "1000base-t" +} + +resource "netbox_ip_address" "test_v4_3" { + ip_address = "1.1.1.18/32" + status = "active" + interface_id = netbox_device_interface.test3.id + object_type = "dcim.interface" +} + +resource "netbox_device_primary_ip" "test_v4_3" { + device_id = netbox_device.test3.id + ip_address_id = netbox_ip_address.test_v4_3.id +}`, testName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("netbox_device.test3", "name", testName), + resource.TestCheckResourceAttr("netbox_device.test3", "tags.#", "1"), + resource.TestCheckResourceAttr("netbox_device.test3", "tags.0", testName), + resource.TestCheckResourceAttr("netbox_device.test3", "status", "planned"), + resource.TestCheckResourceAttrPair("netbox_device_primary_ip.test_v4_3", "device_id", "netbox_device.test3", "id"), + resource.TestCheckResourceAttrPair("netbox_device_primary_ip.test_v4_3", "ip_address_id", "netbox_ip_address.test_v4_3", "id"), + ), + }, + }, + }) +}