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

Ignore Services of type ExternalName in NodePortLocal Controller #3114

Conversation

antoninbas
Copy link
Contributor

For ExternalName Services, the selector is meaningless and should be
ignored. Before this patch, the NPL Controller would treat these
Services in the same way as ClusterIP / LoadBalancer Services.

Signed-off-by: Antonin Bas abas@vmware.com

@antoninbas antoninbas added the area/proxy/nodeportlocal Issues or PRs related to the NodePortLocal feature label Dec 10, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2021

Codecov Report

Merging #3114 (883d7cc) into main (3e869d8) will decrease coverage by 8.42%.
The diff coverage is 30.00%.

❗ Current head 883d7cc differs from pull request most recent head de2186c. Consider uploading reports for the commit de2186c to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3114      +/-   ##
==========================================
- Coverage   59.81%   51.39%   -8.43%     
==========================================
  Files         331      303      -28     
  Lines       28437    36350    +7913     
==========================================
+ Hits        17010    18681    +1671     
- Misses       9549    15881    +6332     
+ Partials     1878     1788      -90     
Flag Coverage Δ
e2e-tests 51.39% <30.00%> (?)
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/nodeportlocal/k8s/npl_controller.go 61.21% <30.00%> (-1.73%) ⬇️
pkg/controller/egress/controller.go 0.00% <0.00%> (-88.45%) ⬇️
pkg/controller/networkpolicy/endpoint_querier.go 4.58% <0.00%> (-86.85%) ⬇️
pkg/agent/util/iptables/lock.go 0.00% <0.00%> (-81.82%) ⬇️
...g/agent/controller/serviceexternalip/controller.go 0.00% <0.00%> (-80.87%) ⬇️
pkg/controller/ipam/antrea_ipam_controller.go 0.00% <0.00%> (-80.29%) ⬇️
pkg/controller/ipam/validate.go 0.00% <0.00%> (-80.00%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam_controller.go 0.00% <0.00%> (-79.77%) ⬇️
...kg/agent/flowexporter/connections/conntrack_ovs.go 0.00% <0.00%> (-77.58%) ⬇️
pkg/controller/externalippool/validate.go 0.00% <0.00%> (-76.20%) ⬇️
... and 322 more

monotosh-avi
monotosh-avi previously approved these changes Dec 13, 2021
tnqn
tnqn previously approved these changes Dec 13, 2021
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

@antoninbas antoninbas added this to the Antrea v1.6 release milestone Jan 26, 2022
For ExternalName Services, the selector is meaningless and should be
ignored. Before this patch, the NPL Controller would treat these
Services in the same way as ClusterIP / LoadBalancer Services.

Signed-off-by: Antonin Bas <abas@vmware.com>
@antoninbas antoninbas dismissed stale reviews from tnqn and monotosh-avi via de2186c January 26, 2022 19:20
@antoninbas antoninbas force-pushed the ignore-ExternalName-Services-in-NPL-controller branch from 895d4cf to de2186c Compare January 26, 2022 19:20
@antoninbas
Copy link
Contributor Author

/test-all

@antoninbas
Copy link
Contributor Author

/test-e2e

@antoninbas antoninbas changed the title Ignore Services of type ExternalName in NoderPortLocal Controller Ignore Services of type ExternalName in NodePortLocal Controller Jan 31, 2022
@antoninbas antoninbas added the action/release-note Indicates a PR that should be included in release notes. label Jan 31, 2022
@antoninbas
Copy link
Contributor Author

Not backporting this unless there is a request to do so, as it is a very minor fix

@antoninbas antoninbas merged commit e23cf3b into antrea-io:main Jan 31, 2022
@antoninbas antoninbas deleted the ignore-ExternalName-Services-in-NPL-controller branch January 31, 2022 23:55
yanjunz97 pushed a commit to yanjunz97/antrea that referenced this pull request Feb 14, 2022
…rea-io#3114)

For ExternalName Services, the selector is meaningless and should be
ignored. Before this patch, the NPL Controller would treat these
Services in the same way as ClusterIP / LoadBalancer Services.

Signed-off-by: Antonin Bas <abas@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes. area/proxy/nodeportlocal Issues or PRs related to the NodePortLocal feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants