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

Automated cherry pick of #3467: Fix duplicate IP case for NetworkPolicy #3475

Conversation

tnqn
Copy link
Member

@tnqn tnqn commented Mar 18, 2022

Cherry pick of #3467 on release-1.2.

#3467: Fix duplicate IP case for NetworkPolicy

For details on the cherry pick process, see the cherry pick requests page.

After a Node restarts, IP allocation storage used by host-local plugin
is reset and all Pods on the Node will be recreated. It could happen
that two Pods have same IP in K8s API if the Pod previously owning the
IP hasn't been recreated and another Pod gets the IP assigned when it's
recreated. This leads to an issue in NetworkPolicy as it calculated IPs
to add and remove by getting added and removed Pods first, then getting
IPs from the Pods.

NetworkPolicy rule reconciler didn't handle this case correctly as it
calculated added and removed Pods first, then got IPs to add and remove
from the added and removed Pods. For example, if both Pod A and Pod B
have IP 1.1.1.1, removing Pod B would cause IP 1.1.1.1 to be removed
from dataplane.

This patch changes to calculate IPs to add and remove based on the
difference between the old IPs and the new IPs directly.

Signed-off-by: Quan Tian <[email protected]>
@tnqn tnqn added the kind/cherry-pick Categorizes issue or PR as related to the cherry-pick of a bug fix from the main branch to a release label Mar 18, 2022
@tnqn tnqn requested a review from xliuxu March 18, 2022 05:54
@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2022

Codecov Report

Merging #3475 (af9d223) into release-1.2 (3912286) will increase coverage by 17.84%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff                @@
##           release-1.2    #3475       +/-   ##
================================================
+ Coverage        42.86%   60.71%   +17.84%     
================================================
  Files              146      285      +139     
  Lines            18351    22432     +4081     
================================================
+ Hits              7866    13619     +5753     
+ Misses            9782     7376     -2406     
- Partials           703     1437      +734     
Flag Coverage Δ
e2e-tests ∅ <ø> (?)
kind-e2e-tests 47.14% <100.00%> (?)
unit-tests 42.86% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/controller/networkpolicy/reconciler.go 76.81% <100.00%> (+2.81%) ⬆️
pkg/apiserver/handlers/webhook/convert_crd.go 2.56% <0.00%> (ø)
pkg/apiserver/handlers/webhook/mutation_labels.go 24.71% <0.00%> (ø)
pkg/agent/cniserver/ipam/ipam_delegator.go 48.83% <0.00%> (ø)
pkg/util/logdir/logdir.go 44.44% <0.00%> (ø)
pkg/legacyapis/system/v1beta1/register.go 90.00% <0.00%> (ø)
...ver/registry/controlplane/nodestatssummary/rest.go 100.00% <0.00%> (ø)
pkg/controller/metrics/prometheus.go 4.34% <0.00%> (ø)
...clusterinformation/v1beta1/antreacontrollerinfo.go 0.00% <0.00%> (ø)
pkg/apiserver/apiserver.go 88.00% <0.00%> (ø)
... and 234 more

@tnqn
Copy link
Member Author

tnqn commented Mar 18, 2022

/test-all

@xliuxu
Copy link
Contributor

xliuxu commented Mar 18, 2022

The e2e failure is caused by unrelated test casesTestFlowAggregator.

@tnqn
Copy link
Member Author

tnqn commented Mar 18, 2022

/skip-e2e

@tnqn tnqn merged commit e703356 into antrea-io:release-1.2 Mar 18, 2022
@tnqn tnqn deleted the automated-cherry-pick-of-#3467-upstream-release-1.2 branch March 18, 2022 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cherry-pick Categorizes issue or PR as related to the cherry-pick of a bug fix from the main branch to a release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants