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

Fix ipBlock referenced in nested ClusterGroup not processed correctly #3383

Merged
merged 1 commit into from
Mar 4, 2022

Conversation

Dyanngg
Copy link
Contributor

@Dyanngg Dyanngg commented Mar 2, 2022

fixes #3376

Signed-off-by: Yang Ding dingyang@vmware.com

@Dyanngg Dyanngg added the area/network-policy/controller Issues or PRs related to the network policy controller label Mar 2, 2022
@Dyanngg Dyanngg added this to the Antrea v1.6 release milestone Mar 2, 2022
@Dyanngg Dyanngg self-assigned this Mar 2, 2022
@Dyanngg Dyanngg changed the title Fix ipBlock referenced in child ClusterGroup not processed correctly Fix ipBlock referenced in nested ClusterGroup not processed correctly Mar 2, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 2, 2022

Codecov Report

Merging #3383 (2d9d1aa) into main (3f30d84) will decrease coverage by 7.20%.
The diff coverage is 43.56%.

❗ Current head 2d9d1aa differs from pull request most recent head 3d8b34c. Consider uploading reports for the commit 3d8b34c to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3383      +/-   ##
==========================================
- Coverage   60.85%   53.64%   -7.21%     
==========================================
  Files         266      239      -27     
  Lines       26520    34211    +7691     
==========================================
+ Hits        16139    18353    +2214     
- Misses       8591    14083    +5492     
+ Partials     1790     1775      -15     
Flag Coverage Δ
e2e-tests 53.64% <43.56%> (?)
kind-e2e-tests ?
unit-tests ?

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

Impacted Files Coverage Δ
pkg/agent/agent.go 50.12% <0.00%> (-1.58%) ⬇️
pkg/agent/apiserver/handlers/ovsflows/handler.go 13.48% <ø> (-61.52%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam_controller.go 0.00% <0.00%> (-79.77%) ⬇️
pkg/agent/controller/networkpolicy/packetin.go 68.34% <ø> (+1.19%) ⬆️
pkg/agent/ipassigner/responder/ndp_responder.go 0.00% <0.00%> (ø)
pkg/agent/openflow/network_policy.go 64.38% <0.00%> (-18.87%) ⬇️
pkg/agent/openflow/pipeline_other.go 3.44% <0.00%> (+1.27%) ⬆️
pkg/agent/types/networkpolicy.go 81.08% <ø> (-2.26%) ⬇️
.../registry/networkpolicy/clustergroupmember/rest.go 21.42% <0.00%> (-66.81%) ⬇️
pkg/controller/egress/controller.go 0.00% <0.00%> (-88.45%) ⬇️
... and 290 more

@tnqn tnqn added action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. labels Mar 2, 2022
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some minor comments

pkg/controller/networkpolicy/networkpolicy_controller.go Outdated Show resolved Hide resolved
test/e2e/antreapolicy_test.go Outdated Show resolved Hide resolved
@Dyanngg
Copy link
Contributor Author

Dyanngg commented Mar 2, 2022

/test-all

@Dyanngg
Copy link
Contributor Author

Dyanngg commented Mar 3, 2022

/test-all

@@ -62,12 +62,11 @@ func (r *REST) Get(ctx context.Context, name string, options *metav1.GetOptions)
effectiveIPBlocks = append(effectiveIPBlocks, ipb.CIDR)
}
memberList.EffectiveIPBlocks = effectiveIPBlocks
} else {
effectiveMembers := make([]controlplane.GroupMember, 0, len(groupMembers))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it still better to keep the code that allocates required size in one shot to be a bit more efficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course. My bad. Fixed

Signed-off-by: Yang Ding <dingyang@vmware.com>
@Dyanngg
Copy link
Contributor Author

Dyanngg commented Mar 4, 2022

/test-all

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@tnqn tnqn merged commit c6926ed into antrea-io:main Mar 4, 2022
@tnqn
Copy link
Member

tnqn commented Mar 4, 2022

@Dyanngg could you backport this PR to previous releases if applicable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. area/network-policy/controller Issues or PRs related to the network policy controller
Projects
None yet
Development

Successfully merging this pull request may close these issues.

acnp does work with IPBlock clustergroup set in from under ingress
3 participants