🐛 Controller does not allocate reserved addresses #171
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/XL
| if gateway != "" { | ||
| addrStrings = append(addrStrings, gateway) | ||
| } | ||
|
|
There was a problem hiding this comment.
We no longer consider the gateway in use, but with this change set the gateway is not in available list.
| Entry("When the addresses range includes network addr, it is not available - InClusterIPPool", | ||
| "InClusterIPPool", 24, []string{"10.0.0.0-10.0.0.1"}, "10.0.0.2", 1, 1, 0), | ||
| Entry("When the addresses range includes broadcast, it is not available - InClusterIPPool", | ||
| "InClusterIPPool", 24, []string{"10.0.0.254-10.0.0.255"}, "10.0.0.1", 1, 1, 0), |
There was a problem hiding this comment.
When a reserved address is in the Spec.Addresses, it is not considered used, nor free.
|
Once again, I worked on this commit, so I should recuse. FWIW, LGTM. @adobley and I re-reviewed it after some time away and we think it's good to go. |
flawedmatrix
force-pushed
the
reserve-network-broadcast-addrs
branch
from
7295e55 to
a5939b1
Compare
|
Fixed a misspelling, should be good to go. |
k8s-ci-robot
added
cncf-cla: no
|
lgtm, but the linter is complaining about one spelling mistake. Sorry that it took so long, a lot going on. |
flawedmatrix
force-pushed
the
reserve-network-broadcast-addrs
branch
from
a5939b1 to
f74b3eb
Compare
k8s-ci-robot
added
cncf-cla: yes
|
Fixed the typo and the CLA fixed itself. Strange. |
unless `pool.Spec.AllocateReservedIPAddresses` is true. The validation code continues to accept pools that contain the network (the first address in the inferred subnet) and broadcast (the last address in the inferred subnet) addresses, but the controller will not allocate these addresses. When the pool is configured with IPv6, the anycast address (the first address in the inferred subnet) will not be allocated. This commit also refactors the claim reconciler. Previously, the controller would continue to reconcile (and ultimately no-op) despite the pool not being found. The new spec flag, `AllocateReservedIPAddresses` exposed this oddness, prompting some refactoring. Co-authored-by: Aidan Obley <[email protected]> Co-authored-by: Edwin Xie <[email protected]>
adobley
force-pushed
the
reserve-network-broadcast-addrs
branch
from
f74b3eb to
96dbef6
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adobley, schrej The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
What this PR does / why we need it:
The validation code continues to accept pools that contain the network (the first address in the inferred subnet) and broadcast (the last address in the inferred subnet) addresses, but the controller will not allocate these addresses. When the pool is configured with IPv6, the anycast address (the first address in the inferred subnet) will not be allocated.
This PR also refactors the claim reconciler. Previously, the controller would continue to reconcile (and ultimately no-op) despite the pool not being found. The new spec flag,
AllocateReservedIPAddressesexposed this oddness, prompting some refactoring.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #165