Bug 1898160: Egress IP fail-over and health checks #394
Conversation
Now that local gateway mode is removed we can cleanup stuff like modeEgressIP and the likes and just use one data container as a controller for egress IP assignment/setup Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> Cleanup egress IP conditions There were a couple of nits on PR: ovn-org/ovn-kubernetes#1668 that I didn't get the chance to fix before it merged. So I am doing that here instead Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
Kubelet status is not enough to be able to determine if a node's networking is up or not. The NotReady status can be assigned to nodes for other reasons other than networking being down and moreover, even when it is correctly indicating the node status, it might not be doing that fast enough. Thus add a recurring connectivity checker to all egress nodes to ascertain if they are up or not. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
Unrelated to the previous commits, this one fixes an update issue during the re-assignment procedure, namely: if a re-assignment is attempted when there are no more nodes to assign to, we previously exited the function `reassignEgressIP` without updating the status. This commit thus changes the execution to continue on to the update phase, yet log the error once the re-assignment is finished. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
Previously we never actually reacted to the fact that an egress IP might have been deleted while we were trying to re-assign it. We should in such a case drop if from our re-assignment cache so that it's not retried anymore. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
Host network pods are not egress IP assignable, since that will break cluster networking and moreover apply the egress IP to all pods running on that node. No check however existed insuring that such pods are skipped for egress assignment. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
This fixes an issue seen when multiple assignment retries are attempted for the same EgressIP object. When multiple nodes are being labelled, we attempt a re-assignment of all EgressIPs which did not get fully assigned in round N-1, however we ended up deleting every EgressIP object once it was retried: thus, ending up retrying every object only once. This patch fixes that, only deleting the EgressIP once all requested .spec.egressIPs have been assigned. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
|
@alexanderConstantinescu: This pull request references Bugzilla bug 1898160, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 6 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
bugzilla/severity-high
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexanderConstantinescu, danwinship 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 |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
russellb
added
cherry-pick-approved
|
(patch manager) 4.6 blocker identified, so pulling unmerged patches from the queue |
@russellb : just for my understanding: you added and then removed the |
crawford
changed the title
crawford
added
the
cherry-pick-approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@alexanderConstantinescu: All pull requests linked via external trackers have merged: Bugzilla bug 1898160 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
- What this PR does and why is it needed
This PR fixes the bug referenced. It brings in several commits (which only touch the egress IP code) which are needed for a smooth back-port. All commits have been cherry-picked without any additional modification.
These commits have fixed the incorrect behavior on master.
/assign @danwinship
- Special notes for reviewers
- How to verify it
- Description for the changelog