[Draft] Allow specification of a desired IP-Address with an IPAddressClaim#131
[Draft] Allow specification of a desired IP-Address with an IPAddressClaim#131Arakos wants to merge 2 commits intotelekom:mainfrom
Conversation
| getInfobloxClientForInstanceFunc = getInfobloxClientForInstance | ||
| newHostnameHandlerFunc = getHostnameResolver | ||
| hostnameAnnotation = "ipam.cluster.x-k8s.io/hostname" | ||
| targetAddressAnnotation = "ipam.cluster.x-k8s.io/address" |
There was a problem hiding this comment.
I would prefer 'desired' instead of 'target'.
Should we also reflect the fact that this is what the consumer wants and not necessarily what he'll get by changing the annotation to ipam.cluster.x-k8s.io/desired-address as well?
There was a problem hiding this comment.
Thought the same thing regarding the name - changed it accordingly.
However I'd treat the presence of the annotation as 'requirement' - so if it is present but the specified IP address cannot be allocated (e.g. it is invalid, not available because there already is a hostrecord allocated to it, etc.) fail and don't produce an IPAddress for the claim until the user either changes the annotations value or the other records consuming the address are released.
Arakos
force-pushed
the
feature/predefined-ip-address
branch
from
bdc22c0 to
4d0512b
Compare
* improve API type validation to actually do as intended + update it to support SSA patches for objects * fix the known issue with infoblox not returning some fields for hostrecords by using the correct request method * add basic infoblox error message parsing (falls back to raw error if not possible) * automatically assert that a hostname for a claim has the DNSZone suffix if a dns zone is set on the referenced IPPool but the hostname does not have the exact zone as suffix * fix makefile to generate any mocks on make generate * fixed controller params to correctly pass cert-key and name params to the related options struct * adapt testcases to any changes
adds an annotation "ipam.cluster.x-k8s.io/desired-address" check to an IPAddressClaim where a specific IP Address can be configured. If present, the address is parsed and the created host record will get this IP assigned if available. If not available the claim will enter an error state.
Arakos
force-pushed
the
feature/predefined-ip-address
branch
from
4d0512b to
4034a4a
Compare
|
@schrej I also noticed some existing issues / inconsistencies in the current codebase. I addressed them in a separate commit before implementing the feature in a follow up commit. I can split the changes in two separate PRs if you prefer to keep fixes/improvements and features separated. |
|
Another caveat: the proposed implementation is 'best effort' only and currently cannot ensure a desired address is not assigned multiple times (at least if DNS is disabled; with DNS enabled, Infoblox will block duplicate creation of DNS records). The reason here is a race condition where two separate requests are required to first search for any existing host record on the desired address and—if there are none—do a second request creating the host record. Here, we have no way of blocking other instances of the controller or other systems from creating a host record on that IP address in the meantime. For the 'old way', the Infoblox "next available" function handled that internally. I currently see no way to do the operation atomically, though. Let me know if you have any idea how to improve that fact. |
| if !strings.HasSuffix(hostname, dnsZone) { | ||
| hostname = fmt.Sprintf("%s.%s", hostname, dnsZone) | ||
| } |
There was a problem hiding this comment.
This new behavior is also up for debate.
Is that a good idea, or should we instead validate that (if a dnsZone is configured) the hostname values must have the correct suffix?
If not, we should return an error, because if we don't, Infoblox will return one with a cryptic message.
Arakos
changed the title
|
PR is on-hold for now |
Arakos
changed the title
schrej
changed the title
2 participants
Sometimes one wishes to claim a specific IP-Address in a Network. The reasons for that are endless, in our case we want to be able to allocate IP Addresses with a specific offset in a subnet (e.g. the first 3 addresses) for special use.
This is currently not possible because the Infoblox IPAM Provider always uses the 'next-avaialble-ip' Infoblox function when allocating a new hostrecord.
This PR introduces an annotation (could be promoted to a spec field as well) which allows to specify an IP-Address to allocate in Infoblox on the IPAddressClaim.
If the IP Address is not already allocated and is part of at least one of the subnets of an IP-Pool the Provider will allocate it accordingly. If the Address is already in use, not part of any subnet of the IPPool etc. an error will be reported and no Addressallocation will happen for the IPClaim.