Skip to content
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

CNI: quit waiting for pod flows for obsoleted ADD #1831

Merged
merged 1 commit into from Nov 12, 2020

Conversation

trozet
Copy link
Contributor

@trozet trozet commented Nov 11, 2020

Given a scenario where we have:

  1. pod A added
  2. pod A deleted
  3. kubelet has already started CNI ADD for first event
  4. pod A added
  5. kubelet starts 2nd sandbox CNI ADD

The current code will remove the original OVS interface for the first
event, however the cmdAdd for the first event will still be running
waiting for flows (up to 20 seconds). Really if hte pod loses its
interface, that cmdAdd iteration is now invalid and we should just
return out immediately with an error.

Signed-off-by: Tim Rozet trozet@redhat.com

@coveralls
Copy link

coveralls commented Nov 11, 2020

Coverage Status

Coverage decreased (-0.05%) to 55.124% when pulling edd836e on trozet:quit_waiting_for_flows1 into a2a8b10 on ovn-org:master.

go-controller/pkg/cni/ovs.go Outdated Show resolved Hide resolved
Given a scenario where we have:
1. pod A added
2. pod A deleted
3. kubelet has already started CNI ADD for first event
4. pod A added
5. kubelet starts 2nd sandbox CNI ADD

The current code will remove the original OVS interface for the first
event, however the cmdAdd for the first event will still be running
waiting for flows (up to 20 seconds). Really if hte pod loses its
interface, that cmdAdd iteration is now invalid and we should just
return out immediately with an error.

Signed-off-by: Tim Rozet <trozet@redhat.com>
@dcbw
Copy link
Contributor

dcbw commented Nov 12, 2020

/lgtm now

@trozet trozet merged commit c45e7ef into ovn-org:master Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants