Excess IP release handshake 1.8 backport#81
Merged
hemanthmalla merged 8 commits into1.8.x-ddfrom Jan 6, 2022
Merged
Conversation
Currently there's a potential 15 sec delay in updating ciliumnode CRD after IP allocation to a pod, meanwhile the operator can determine that a node has excess IPs and release the IP causing pod connectivity issues. A new operator flag `excess-ip-release-delay` is added to control how long operator should wait before considering an IP for release(defaults to 180 secs). This is done to better handle IP reuse during rollouts. Operator and agent use a new map in cilium node status .status.ipam.release-ips to exchange information during the handshake. Fixes: cilium#13412 Signed-off-by: Hemanth Malla <[email protected]>
After the handshake is complete and the operator is done releasing an IP, CiliumNode status (release-ips) and spec (pool) are updated in two consecutive requests. There's a tiny window between the two updates where the entry is removed from .status.ipam.release-ips but the IP is still present in spec.ipam.pool. It was possible that the IP could be allocated between these requests. This commit introduces a new state called released to deal with this. Now agent removes the entry from release-ips only when the IP was removed from .spec.ipam.pool as well. Signed-off-by: Hemanth Malla <[email protected]>
Fixes: cilium#18204 Signed-off-by: Hemanth Malla <[email protected]>
* Fix for blocked state transition from ready-for-release to released * Fix for unnecessary updates between agent and operator during handshake Signed-off-by: Hemanth Malla <[email protected]>
crdAllocator.Allocate() aquires lock on allocator first and then on nodestore. But updateLocalNodeResource() acquires locks in the opposite order. This commit releases nodestore lock before acquiring allocator lock to avoid potential deadlocks due to inconsistent lock ordering. Signed-off-by: Hemanth Malla <[email protected]>
46035d2 to
e635317
Compare
… node Imagine a scenario where a node has 2 unused IPs and pre-allocate set to 1. Let's say one of the IPs is in the middle of a handshake and a new pod is scheduled on the node. The other unused IP would be allocated to the pod. Now, when the operator re-evaluates, the node is no longer considered to be in excess. Without this commit, the operator does not act further on IPs in this state. This results in a scenario where no new IPs are allocated to the node and agent cannot allocate the unused IPs because they're in the middle of a handshake. Signed-off-by: Hemanth Malla <[email protected]>
…ol() Signed-off-by: Hemanth Malla <[email protected]>
Signed-off-by: Hemanth Malla <[email protected]>
8a60d01 to
36a4268
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of cilium#17939, cilium#18217 and cilium#18330 to cilium 1.8.7.
See commit and PR description for more details.