HBO: Make GetPortAddresses return portMAC even if portIP is nil #2395
Conversation
This file contains 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
In GetPortAddresses, we check if we have both PortMAC and IP for the port. If not we return nil for both values. While this works for normal ports, for the HBO ports which don't have IPs for the int-<node> interface, we will always hit this condition and return an empty PortMAC which in turn triggers the lsp-add even if the portMAC already exists. When we have an external entity updating the node object, the watcher triggers handleOverlayPort and we keep hitting this condition. Let's remove the "len(addresses)<2" check since its not needed for HBO and in normal pod port case, pods.getPortAddresses already handles the case for empty podMac or podIPs. This way when portMAC is set but not the IPs, we will still return the portMAC and the respective code paths handle it as needed. Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
|
/assign @dcbw |
tssurya
changed the title
|
/lgtm |
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.
- What this PR does and why is it needed
In
GetPortAddresses, we check if we have both PortMAC and IP forthe port. If not we return nil for both values. While this works
for normal ports, for the HBO ports which don't have IPs for
the
int-<node>interface, we will always hit this condition andreturn an empty PortMAC which in turn triggers the lsp-add even
if the portMAC already exists.
When we have an external entity updating the node object, the
watcher triggers
handleOverlayPortand we keep hitting thiscondition.
Let's remove the
len(addresses)<2check since its not neededfor HBO and in normal pod port case,
pods.getPortAddressesalreadyhandles the case for empty podMac or podIPs. This way when portMAC
is set but not the IPs, we will still return the portMAC and the
respective code paths handle it as needed.
Signed-off-by: Surya Seetharaman suryaseetharaman.9@gmail.com
- How to verify it
Spin up a KIND cluster. Before the PR we could see lots of:
With the PR, we handle this better for node updates since now
GetPortAddresseswill return the portMAC even if portIPs are nil.