-
Notifications
You must be signed in to change notification settings - Fork 455
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
Extend ignored_guest_ips to support CIDR networks #841
Conversation
Patch updated following testing. Is there anything else required from me before this can be merged? We need the functionality in order to prevent the provider from binding to the temporary DHCP IP temporarily detected even when we assign a fixed IP via Guest Customization, for example if the attached subnet hands out dynamic addresses in resource "vsphere_virtual_machine" "foo" {
…
ignored_guest_ips = ["10.0.0.128/25"]
clone {
…
customize {
…
network_interface {
ipv4_address = 10.0.0.10
ipv4_netmask = 24
}
}
}
} |
* Correctly parse CIDR networks included in the `ignored_guest_ips` list, allowing entire arbitrary IP ranges to be ignored. * Enhance `ignored_guest_ips` comparison to ignore differences in string formatting (particularly useful for IPv6 addresses).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @isometry,
First of all, thanks for taking the time to submit the PR. The feature you introduce looks useful. I have a comment around the validation of the CIDR strings, but other than that it looks OK.
vsphere/internal/helper/virtualmachine/virtual_machine_helper.go
Outdated
Show resolved
Hide resolved
@koikonom : I'm not sure why this PR is still marked "Changes Requested", but I implemented the changes you suggested two weeks ago. Any chance of getting this merged before v1.14.0? |
Hi @isometry, I am sorry for the delay. We have been having some issues with our acceptance tests lately so I've spent all my time getting these in order first. Once we are confident that things are back in working order I will merge this PR (before v1.14.0). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ignored_guest_ips
list, allowing entire arbitrary IP ranges to be ignored.ignored_guest_ips
usingnet.IP.Equal()
rather than raw string comparison, enhancing support for variations in IPv6 representation.