-
Notifications
You must be signed in to change notification settings - Fork 18
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
Try to release IP if it has been allocated to current LB #47
Conversation
To solve the duplicated allocation error when LB is frequently created and deleted. Signed-off-by: Jian Wang <[email protected]>
f2219b9
to
d95dfea
Compare
Test plan: (1) Create an IP pool
(1) Set an old image tag When creating & deleting a same object frequently, it will trigger the bug harvester/harvester#7449
The pod
(2) Use new image (master-head), it automatically release the complained duplicated allocated IP
(3) Manaully edit pool1 object to add following annotation, and observe the pod log
(4) Repeat (1) to reproduce the error, and then delete the LB object,
(5) Replace the LB with new image, manaully edit pool1 object to add following annotation (change to IP), and observe the pod log
The pod should have such log
check the pool, the IP was released
|
Signed-off-by: Jian Wang <[email protected]>
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.
Overall LGTM. Leave a minor comment. Thanks.
|
||
a := h.allocatorMap.Get(ipPool.Name) | ||
if a == nil { | ||
return ipPool, fmt.Errorf("IP Pool %s has a manual IP release request %s, fail to get allocator", ipPool.Name, ipStr) |
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.
If there is an IPPool, but there is no internal allocator, can we just create a new allocator for it and leave some log? If we return error directly, the controller may keep retrying.
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.
There has already been a OnChange
controller, which ensures the allocator.
a, err := ipam.NewAllocator(ipPool.Name, ipPool.Spec.Ranges, h.ipPoolCache, h.ipPoolClient) |
If this place happens to get a nil allocator, then next reconciller will get it normally. That's the consideration, thanks.
@mergify backport v1.5 |
✅ Backports have been created
|
To solve the duplicated allocation error when LB is frequently created and deleted.
In case LB is repeatedly created and deleted, the deletion onRemove may not be called due to UUID change on the same namespace/name object.
Solution:
issue:
harvester/harvester#7449
Test plan: #47 (comment)