Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate IPs when creating multiple netbox_available_ip_address resources #59

Closed
holmesb opened this issue Jul 7, 2021 · 12 comments
Closed

Comments

@holmesb
Copy link
Contributor

holmesb commented Jul 7, 2021

Terraform Version

terraform v0.13.2
provider v0.2.0
netbox: v2.11.7

Affected Resource(s)

netbox_available_ip

Terraform Configuration Files

provider "netbox" {
  server_url           = "https://netbox.domain.com"
  api_token            = var.netbox_api_token
  allow_insecure_https = false
}
resource "netbox_prefix" "main" {
  prefix = "10.0.0.0/24"
  status = "active"
  description = "main"
}
resource "netbox_cluster_type" "main" {
  name = "main"
}
resource "netbox_cluster" "main" {
  cluster_type_id = netbox_cluster_type.main.id
  name = "main"
}
resource "netbox_virtual_machine" "main" {
  cluster_id = netbox_cluster.main.id
  name = "main"
}
resource "netbox_interface" "main" {
  name = "main"
  virtual_machine_id = netbox_virtual_machine.main.id
}
resource "netbox_available_ip_address" "main1" {
  prefix_id = netbox_prefix.main.id
  status = "active"
  interface_id = netbox_interface.main.id
}
resource "netbox_available_ip_address" "main2" {
  prefix_id = netbox_prefix.main.id
  status = "active"
  interface_id = netbox_interface.main.id
}

Expected Behavior

Two distinct IP addresses created. 10.0.0.1/24 & 10.0.0.2/24

Actual Behavior

2 x 10.0.0.1/24

Important Factoids

If I delete netbox_available_ip_address.main2 (comment it out and apply), then recreate it, it will correctly create a fresh IP: 10.0.0.2/24. So I suspect a race-condition when multiple are created in the same execution.

Might be of interest to @pezhore

@fbreckle
Copy link
Collaborator

fbreckle commented Jul 7, 2021

My first guess would also be a race condition. I'll add a testcase and see if I can reproduce the error.
If its just a race condition, a possible workaround could be adding a small artificial wait time after getting an IP.

@fbreckle
Copy link
Collaborator

fbreckle commented Jul 7, 2021

Ok I also checked the code for the available IP resource. It basically uses the api to get ALL available IPs in the prefix, then picks the first one and creates that.
The problem is probably this: Since in your code, the IPs do not have a dependency on each other, terraform parallelizes the creation of the resources. This means it will do the things above twice simultaneously which will result in the same IP.

To verify this, you could try two workarounds:
1.) use the -parallelism=1 option of terraform to force linear creation of resources.
2.) add depends_on = [netbox_available_ip_address.main1] to the main2 resource to artificially create a dependency, and thus sequence, between the two IP addresses.

Both these workarounds are not ideal and might not scale well. With the way the resource is implemented right now, I do not see any other options.

@fbreckle
Copy link
Collaborator

fbreckle commented Jul 7, 2021

On a side-note, this netbox-community/netbox#1246 discusses (and links the implementation) the endpoint that I actually expected to exist: One where you can just grab the next available IP from netbox and netbox handles the rest. Maybe we can rework the resource to use that instead.

@holmesb
Copy link
Contributor Author

holmesb commented Jul 8, 2021

We can live with -parallelism=1 for the time being, doesn't slow things down too much.

@dstoffel
Copy link

dstoffel commented Aug 4, 2021

Hi,

Same issue here.

-parallelism=1 : not really an option for us, a lot of dependencies and resource to deploy, considerately increase the execution.
depends_on : we rely on "count" for most of the managed resources and depends_on can only be a static variable reference, not possible to do calculation to force the dependencies on "count.index - 1" :(

Any other workaround in mind ? I don't find another one by myself.

thanks!

@matjazp
Copy link

matjazp commented Sep 6, 2021

This should be reimplemented with more suitable API calls (already mentioned /api/ipam/prefixes/[prefix id]/available-ips/).

In the mean time, my workaround was to use generic REST Terraform provider (e.g. Mastercard's restapi) to "get-and-reserve" one free IP. But this is far from optimal.

holmesb added a commit to holmesb/terraform-provider-netbox that referenced this issue Jan 7, 2022
holmesb added a commit to holmesb/terraform-provider-netbox that referenced this issue Jan 7, 2022
holmesb added a commit to holmesb/terraform-provider-netbox that referenced this issue Jan 7, 2022
holmesb added a commit to holmesb/terraform-provider-netbox that referenced this issue Jan 17, 2022
@stanier
Copy link

stanier commented Apr 4, 2022

@fbreckle was this ever tested after dfcbba3? I am experiencing the same issue described here. When testing with four IPs/VMs, it works fine but when I step it up to eight there will start being duplicates.

--parallelism=1 is not an option for us as each provisioning pass takes 10+ minutes, which we are trying to shave down, we can't afford to have it approach 45+ minutes for 4 VMs.

@holmesb
Copy link
Contributor Author

holmesb commented Apr 5, 2022

@stanier, I've tried eight and 12, and can't reproduce in my v1.5.2. No duplicates when creating in either a prefix or range. Pls can you type terraform providers and check you're on latest version, and if so post your code

@stanier
Copy link

stanier commented Apr 6, 2022

@holmesb I am using v1.5.2. Could it be dependent on the version of Netbox?

I'll see if I can find the code that was producing duplicates, but I actually just got done rewriting the functionality as a python heredoc, which solves the issue:

resource "null_resource" "get_available_ips" {
    provisioner "local-exec" {
        command = <<-EOF
        import pynetbox
        import os

        nb = pynetbox.api(
            "${var.netbox_server}",
            token="${var.netbox_api_token}"
        )

        prefix = nb.ipam.prefixes.get(prefix="${var.ip_prefix}")

        ips = prefix.available_ips.create([{} for i in range(${var.vm_count})])

        if os.path.exists('.availableips'):
            os.remove('.availableips')

        with open('.availableips', 'w') as f:
            print(ips, file=f)
        EOF
        interpreter = ["python3", "-c"]
    }
}

data "local_file" "ips_file" {
    depends_on = [
        null_resource.get_available_ips
    ]
    filename = "${path.module}/.availableips"
}

locals {
    available_ips = split(", ", trim(data.local_file.ips_file.content,"[]\n"))
}

@stanier
Copy link

stanier commented Apr 7, 2022

@holmesb here are the resources I had issues with


resource "netbox_virtual_machine" "nb_test_vm" {
    count        = var.vm_count
    name         = "${var.vm_name}${count.index + 1}"
    cluster_id   = data.netbox_cluster.devcluster.id
    disk_size_gb = var.vm_disksizegb
    memory_mb    = var.vm_mem
    vcpus        = "${var.vm_cpu}"
}

resource "netbox_interface" "testvm-eth0" {
    depends_on = [
        netbox_virtual_machine.nb_test_vm
    ]

    count              = var.vm_count
    name               = "eth0"
    virtual_machine_id = netbox_virtual_machine.nb_test_vm[count.index].id
}

resource "netbox_available_ip_address" "availableip" {
    depends_on = [
        netbox_interface.testvm-eth0
    ]

    count        = var.vm_count
    prefix_id    = data.netbox_prefix.devclusterprefix.id
    status       = "active"
    interface_id = netbox_interface.testvm-eth0[count.index].id
}

@holmesb
Copy link
Contributor Author

holmesb commented Apr 10, 2022

@holmesb here are the resources I had issues with


resource "netbox_virtual_machine" "nb_test_vm" {

...

That code works fine for me on v1.5.2 & Netbox v3.1.4. I also tried v1.6.1 & Netbox v3.1.9. vm_count = 8, 12 and finally 24 successfully created (non-duplicate IPs). Using a dummy prefix 10.0.1.0/24.

@stanier
Copy link

stanier commented Apr 21, 2022

Thanks, I'll look into experimenting with it some in the near future. The python script fix, though hacky, seems to be keeping things working for the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants