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

Create netbox_available_ip_address in IP range and fix duplicates #106

Merged
merged 4 commits into from
Jan 20, 2022

Conversation

holmesb
Copy link
Contributor

@holmesb holmesb commented Jan 7, 2022

Fixed duplicates when creating available IPs (#59), and they can now be created in netbox_ip_ranges (#105). Creation of netbox_available_ip_address resources is now done by making a single call to the native go-netbox IpamIPRangesAvailableIpsCreate method instead of reading spare IPs into an array and then separately creating the first one.

Required a couple of minor changes to the go-netbox client module. Currently this PR uses my forked module to allow it to be tested: https://github.com/holmesb/go-netbox. I propose this PR should be reviewed in conjunction with my PR to https://github.com/fbreckle/go-netbox, and when the go-netbox one is reviewed\merged, I will revert go-netbox references to fbreckle before this is merged. That way we are still using the single fbreckle go-netbox client. Hence pls don't merge this yet!

I've also added a new description attribute to netbox_available_ip_address and added the new slaac status option.

There are a few other module dependency version bumps that go mod tidy decided were necessary, probably because I'm using the latest terraform\go

Edit1: is a broken views glitch with netbox v3.1.3, so you might need to upgrade to v3.1.4 to view IPs using the webui.

Edit2: To avoid having to switch go-netbox client en-masse during PRs when changes to it are needed, might be worth considering moving the client into this repo. That way the client and terraform provider are kept in-sync and the PR process is easier. fbreckle/go-netbox exclusively serves this provider. I'll propose this in a separate issue.

@fbreckle
Copy link
Collaborator

Can you please refactor this as discussed in #107 ?

@holmesb
Copy link
Contributor Author

holmesb commented Jan 11, 2022

When I reset all the required modules back to fbreckle, I can then replace with the local source code dir on my workstation:
replace github.com/fbreckle/go-netbox => ~/go-netbox_holmesb and build this provider fine. You can observe this if you clone my repo on your workstation and checkout the bh/available_ips_changes branch.

But when I try to replace with the github path:
replace github.com/fbreckle/go-netbox => github.com/holmesb/go-netbox v0.0.1, go mod tidy complains:
go: github.com/holmesb/go-netbox@v0.0.1 used for two different module paths (github.com/fbreckle/go-netbox and github.com/holmesb/go-netbox). Not sure, but might be this open issue. I'll dig further into this when I get a chance.

Alternatively, if you're happy with this PR as-is, can merge my go-netbox PR then I can swap all required modules back to fbreckle pointing at the new commit reference. Won't break existing versions of this provider since they point at the prior commit reference.

@fbreckle
Copy link
Collaborator

yeah, the primary reason I have not looked or commented on this is this being a somewhat busy week and also it looks a bit more complicated than usual. If I get to it, I will get it to work one way or the other.

As for your error: do you still have a reference to your fork still somewhere?

@holmesb
Copy link
Contributor Author

holmesb commented Jan 11, 2022

"do you still have a reference to your fork still somewhere?" - search says no. Only reference is go.mod's replace line.

@fbreckle
Copy link
Collaborator

I merged the upstream change in fbreckle/go-netbox

@holmesb
Copy link
Contributor Author

holmesb commented Jan 17, 2022

Ok back on your go-netbox client.

The TestAccNetboxAvailableIPAddress_multipleIpsParallel test was flapping because it's impossible to predict which resource will receive a given IP when they're created in parallel. I've simplified this to only check an IP attribute was created at all. Would be better to check they are all 1) members of a set and 2) different (non-duplicates), but we're probably looking at creating a custom check function. Good enough for now IMO.

@fbreckle fbreckle merged commit dfcbba3 into e-breuninger:master Jan 20, 2022
@holmesb holmesb deleted the bh/available_ip branch April 16, 2022 15:36
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

Successfully merging this pull request may close these issues.

None yet

2 participants