[release-4.7] Bug 1959737: Fix: egress IP route health check detection state on restart #538
Conversation
Egress IP initializes a "watcher" which health checks connectivity to egress nodes every 5 seconds. This is done in its sync function. However, due to a bug in the state initialization in WatchEgressNodes we don't set the different egress node fields correctly in our ADD func which screws up the health check detection mechanism upon a ovnkube-master restart. Essentially it does like this: 1) egress setup is made 2) there's a problem with connectivity to the egress node(s) 3) ovnkube-master is restarted Once step 3) is made: it is expected that ovnkube-master will perform health checks continously and detect when the connectivity in step 2) is back up. However because of this problem that is not the case and even if further connectivity is successful, we never flag the node as such in our internal data. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
openshift-ci
bot
added
bugzilla/severity-medium
|
@alexanderConstantinescu: This pull request references Bugzilla bug 1959737, 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
No GitHub users were found matching the public email listed for the QA contact in Bugzilla (anusaxen@redhat.com), skipping review request. 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. |
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexanderConstantinescu, dcbw 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 |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexanderConstantinescu, dcbw 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
bot
added
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
12 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. |
|
/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. |
|
/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. |
|
/override ci/prow/e2e-aws-ovn-windows |
|
@trozet: Overrode contexts on behalf of trozet: ci/prow/e2e-aws-ovn-windows 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. |
|
/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. |
12 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. |
|
/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. |
|
/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. |
sdodson
added
the
cherry-pick-approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
7 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. |
|
/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. |
|
/override ci/prow/e2e-vsphere-ovn |
|
@trozet: Overrode contexts on behalf of trozet: ci/prow/e2e-vsphere-ovn 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. |
|
@alexanderConstantinescu: All pull requests linked via external trackers have merged: Bugzilla bug 1959737 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. |
|
@sdodson, FYI: I asked @trozet to override the vsphere job. It's been almost perma failing for a while, see: https://prow.ci.openshift.org/job-history/gs/origin-ci-test/pr-logs/directory/pull-ci-openshift-ovn-kubernetes-release-4.7-e2e-vsphere-ovn and this change/feature is not tested in CI, so it could not have impacted the CI tests. |
|
/cherry-pick release-4.6 |
|
@alexanderConstantinescu: new pull request created: #576 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. |
Egress IP initializes a "watcher" which health checks connectivity to egress
nodes every 5 seconds. This is done in its sync function. However, due to a
bug in the state initialization in WatchEgressNodes we don't set the
different egress node fields correctly in our ADD func which screws up the health
check detection mechanism upon a ovnkube-master restart.
Essentially it does like this:
Once step 3) is made: it is expected that ovnkube-master will perform
health checks continously and detect when the connectivity in step 2) is
back up. However because of this problem that is not the case and even
if further connectivity is successful, we never flag the node as such in
our internal data.
Signed-off-by: Alexander Constantinescu aconstan@redhat.com
/assign @dcbw
- What this PR does and why is it needed
- Special notes for reviewers
- How to verify it
- Description for the changelog