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 log spam when there is any DNS based LoadBalancer Service #4234

Merged
merged 1 commit into from
Sep 21, 2022

Conversation

tnqn
Copy link
Member

@tnqn tnqn commented Sep 16, 2022

IP in Service.Status.LoadBalancer.Ingress is optional and may be not set for load-balancer ingress points that are DNS based. It should check if the IP is empty before appending it to the slice, otherwise every round of sync will produce one error log "Skipping invalid IP:" for each IP, leading to log spam.

The same issue has been fixed in K8s upstream by
kubernetes/kubernetes#101690.

Signed-off-by: Quan Tian qtian@vmware.com

Fix #4233

IP in Service.Status.LoadBalancer.Ingress is optional and may be not set
for load-balancer ingress points that are DNS based. It should check if
the IP is empty before appending it to the slice, otherwise every round
of sync will produce one error log "Skipping invalid IP:" for each IP,
leading to log spam.

The same issue has been fixed in K8s upstream by
kubernetes/kubernetes#101690.

Signed-off-by: Quan Tian <qtian@vmware.com>
@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 Sep 16, 2022
@@ -286,8 +289,10 @@ func NewServiceChangeTracker(makeServiceInfo makeServicePortFunc,
// otherwise return false. Update can be used to add/update/delete items of ServiceChangeMap. For example,
// Add item
// - pass <nil, service> as the <previous, current> pair.
//
Copy link
Member Author

Choose a reason for hiding this comment

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

The following change is by gofmt, guess related to go1.19.

@codecov
Copy link

codecov bot commented Sep 16, 2022

Codecov Report

Merging #4234 (2bce100) into main (f0c8170) will increase coverage by 4.21%.
The diff coverage is n/a.

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

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4234      +/-   ##
==========================================
+ Coverage   60.34%   64.55%   +4.21%     
==========================================
  Files         385      361      -24     
  Lines       54456    52188    -2268     
==========================================
+ Hits        32862    33691     +829     
+ Misses      19130    16052    -3078     
+ Partials     2464     2445      -19     
Flag Coverage Δ *Carryforward flag
e2e-tests 39.44% <ø> (?)
integration-tests 34.83% <ø> (+0.03%) ⬆️ Carriedforward from 0d98ab1
kind-e2e-tests 48.97% <ø> (+6.57%) ⬆️ Carriedforward from 0d98ab1
unit-tests 45.06% <ø> (+1.20%) ⬆️ Carriedforward from 0d98ab1

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
pkg/features/antrea_features.go 8.00% <0.00%> (-52.00%) ⬇️
cmd/antrea-agent/options.go 0.00% <0.00%> (-21.23%) ⬇️
pkg/agent/controller/networkpolicy/reject.go 71.00% <0.00%> (-6.50%) ⬇️
pkg/util/env/env.go 58.73% <0.00%> (-4.77%) ⬇️
pkg/antctl/runtime/runtime.go 33.33% <0.00%> (-3.34%) ⬇️
...ntrollers/multicluster/serviceexport_controller.go 76.62% <0.00%> (-2.86%) ⬇️
pkg/apiserver/storage/ram/watch.go 90.66% <0.00%> (-2.67%) ⬇️
...catesigningrequest/ipsec_csr_signing_controller.go 61.65% <0.00%> (-2.46%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam.go 51.08% <0.00%> (-1.74%) ⬇️
...trollers/multicluster/resourceexport_controller.go 75.13% <0.00%> (-0.82%) ⬇️
... and 73 more

Copy link
Contributor

@hongliangl hongliangl left a comment

Choose a reason for hiding this comment

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

LGTM

@hongliangl
Copy link
Contributor

/test-all

@tnqn tnqn merged commit ca89836 into antrea-io:main Sep 21, 2022
@tnqn tnqn deleted the fix-util-log branch September 21, 2022 12:32
heanlan pushed a commit to heanlan/antrea that referenced this pull request Mar 29, 2023
…-io#4234)

IP in Service.Status.LoadBalancer.Ingress is optional and may be not set
for load-balancer ingress points that are DNS based. It should check if
the IP is empty before appending it to the slice, otherwise every round
of sync will produce one error log "Skipping invalid IP:" for each IP,
leading to log spam.

The same issue has been fixed in K8s upstream by
kubernetes/kubernetes#101690.

Signed-off-by: Quan Tian <qtian@vmware.com>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Antrea agent log is full of "Skipping invalid IP:"
2 participants