Avoid printing some service comments in iptables rules #65755
Conversation
wojtek-t
added
sig/network
|
@wojtek-t: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. 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
size/M
k8s-ci-robot
added
the
approved
wojtek-t
added
release-note-none
wojtek-t
force-pushed
the
optimize_kube_proxy
branch
from
c67b885
to
b090498
Compare
|
aww, I use this all the time. I guess it seems like a reasonable tradeoff... We probably should add the flag. |
Another option that I just came up with is to make it depend on the size of iptables. If they are small, we don't really care. And we can just avoid adding those comments in very large iptables. |
wojtek-t
force-pushed
the
optimize_kube_proxy
branch
from
b090498
to
bbd0a98
Compare
|
@thockin - changed. PTAL if that is acceptable for you. |
|
/test pull-kubernetes-e2e-kops-aws |
|
/retest |
|
Just a bit worried about debugability here; when the number of rules is > 1000, how would I match up a given rule with a specific service? |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thockin, wojtek-t 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 |
With big enough iptables (maybe we should tune this number), I'm not sure they are super debuggable anyway... In the worst case, it's possible to connect pods with services and their ips and probably add those comments offline. Maybe a script for that is what we should also add? |
|
Automatic merge from submit-queue (batch tested with PRs 65882, 65896, 65755, 60549, 65927). If you want to cherry-pick this change to another branch, please follow the instructions here. |
This will make "appendServiceCommentLocked" do what it looks like it does. The current implementation is just plain dead code, as the append func does not edit in place. As discussed in the original PR, we could create a new flag to allow the user to enable the comments when needed. This will increase the size of the iptables rules, but not the way they work. Original PR: kubernetes#65755
According to some profiles, with large number of endpoints in the system, comments mentioning the service in appropriate iptables rules may be responsible for 40% of all iptables contents.
Given that ~70% of memory usage of kube-proxy seems to be because of generated iptables rules, the overall saving may be at the level of 30% or so.
OTOH, we sacrifise a bit understandability of iptables, but this PR only changes some of iptables that contribute to the most painful rules.
@thockin @danwinship @dcbw - thoughts?
Ref #65441