Skip to content

Commit

Permalink
fix primary ip race condition on update
Browse files Browse the repository at this point in the history
  • Loading branch information
thibaultbustarret-ovhcloud committed Jul 3, 2024
1 parent 7a3a4f6 commit 1dbf18f
Show file tree
Hide file tree
Showing 2 changed files with 268 additions and 12 deletions.
12 changes: 0 additions & 12 deletions netbox/resource_netbox_device.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
268 changes: 268 additions & 0 deletions netbox/resource_netbox_device_primary_ip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
),
},
},
})
}

0 comments on commit 1dbf18f

Please sign in to comment.