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
Bug 1779163: CNI: Don't wait for missing pods on DEL #280
Bug 1779163: CNI: Don't wait for missing pods on DEL #280
Conversation
@MaysaMacedo: This pull request references Bugzilla bug 1779163, which is invalid:
Comment 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. |
/bugzilla refresh |
@MaysaMacedo: This pull request references Bugzilla bug 1779163, which is invalid:
Comment 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. |
/bugzilla refresh |
@MaysaMacedo: This pull request references Bugzilla bug 1779163, which is invalid:
Comment 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. |
/test verify |
/retest |
/test unit |
/bugzilla refresh |
@luis5tb: This pull request references Bugzilla bug 1779163, which is invalid:
Comment 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. |
/test verify |
/test unit |
1 similar comment
/test unit |
/bugzilla refresh |
@luis5tb: This pull request references Bugzilla bug 1779163, which is valid. 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. |
/lgtm |
/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. |
/retest Please review the full test history for this PR and help us cut down flakes. |
13 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. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
1f06e93
to
90ddd40
Compare
We've seen some issues related to the fact that on CNI DEL we wait for pod annotation to appear in CNI registry: 1. Overloaded kuryr-daemon HTTP server, because many stale DEL requests from CRI will saturate the number of free connections, causing CNI to answer requests extremely slowly. 2. Some CRI's will not delete network namespace before getting a successful CNI DEL. If we'll assign the same IP (or vlan) to a next pod, we might end up with an IP or vlan conflict and NetlinkError. This commit makes sure we only wait for VIFs up to 5 seconds on DEL requests, enough time for watcher to populate registry on restart. Also some code got moved around to make implementating the above simpler and some logs got clarified to make debugging of such issues easier later on. Change-Id: I9221b6bc9166597837a4b53382862aa6c6f3e94c Closes-Bug: 1882083 Related-Bug: 1854928
90ddd40
to
586d639
Compare
/test images |
2 similar comments
/test images |
/test images |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: luis5tb, MaysaMacedo 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 |
@MaysaMacedo: All pull requests linked via external trackers have merged: openshift/kuryr-kubernetes#280, openshift/kuryr-kubernetes#246, openshift/kuryr-kubernetes#182, openshift/kuryr-kubernetes#119. Bugzilla bug 1779163 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. |
Fixes proxy validation error formatting
We've seen some issues related to the fact that on CNI DEL we wait for
pod annotation to appear in CNI registry:
from CRI will saturate the number of free connections, causing CNI to
answer requests extremely slowly.
successful CNI DEL. If we'll assign the same IP (or vlan) to a next
pod, we might end up with an IP or vlan conflict and NetlinkError.
This commit makes sure we only wait for VIFs up to 5 seconds on DEL
requests, enough time for watcher to populate registry on restart. Also
some code got moved around to make implementating the above simpler and
some logs got clarified to make debugging of such issues easier later
on.
Change-Id: I9221b6bc9166597837a4b53382862aa6c6f3e94c
Closes-Bug: 1882083
Related-Bug: 1854928