optimize proxier duplicate localaddrset #98083
Conversation
k8s-ci-robot
added
kind/cleanup
|
@JornShen: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
needs-triage
k8s-ci-robot
added
area/ipvs
sig/network
|
ref issue #91236 |
|
/test pull-kubernetes-e2e-kind-ipv6 |
k8s-ci-robot
added
release-note-none
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/cc @thockin PTAL :) |
JornShen
force-pushed
the
optimize_proxier_duplicate_localaddrset
branch
from
e23af44
to
3f506ca
Compare
k8s-ci-robot
removed
the
lgtm
| @@ -1007,6 +992,12 @@ func (proxier *Proxier) syncProxyRules() { | |||
| proxier.endpointChainsNumber += len(proxier.endpointsMap[svcName]) | |||
| } | |||
|
|
|||
| localAddrSet := utilproxy.GetLocalAddrSet() | |||
There was a problem hiding this comment.
When you put these right next to each other, they seem to overlap a bit. We should clean this up. Later.
There was a problem hiding this comment.
which place you think it is overlap? :)
| // If failed to get local addr, will assume no local ips. | ||
| func GetLocalAddrSet() utilnet.IPSet { | ||
| localAddrs, err := GetLocalAddrs() | ||
| if err != nil { |
There was a problem hiding this comment.
I'm torn. Part of me feels like this should be logged from the caller, which reduces this function to just a few LOC. I guess it doesn't matter much, its an internal API..
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JornShen, thockin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
Both seem to read the list of interface addresses. One filters that
through a list of CIDRs, the other doesn't.
…On Thu, Jan 28, 2021 at 11:42 PM Canhui Shen ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pkg/proxy/iptables/proxier.go
<#98083 (comment)>
:
> @@ -1007,6 +992,12 @@ func (proxier *Proxier) syncProxyRules() {
proxier.endpointChainsNumber += len(proxier.endpointsMap[svcName])
}
+ localAddrSet := utilproxy.GetLocalAddrSet()
which place you think it is overlap? :)
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#98083 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABKWAVH2HWCP5LYODMQBTNTS4JRFTANCNFSM4WDLFYZQ>
.
|
|
failed job: in kube-apiserver can see this log: but node 's kubelet seems still ok at that time seems network connection problems /test pull-kubernetes-e2e-gce-ubuntu-containerd |
filter through cidr, I think it is a validation of output data. I know, now I just pull out the same operation to method But I worry about the method will do much more thing if we merge them. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
syncProxyRulesin proxier.go of iptables and ipvs, there is a same code.At the same time, this code is in the front of
syncProxyRules. Every time we should run it. But in fact, the count resultlocalAddrSetis only use in Capture externalIPskubernetes/pkg/proxy/iptables/proxier.go
Line 1102 in da26de3
So I migrate it to the utils module.
Does this PR introduce a user-facing change?: