detectNumCPU sized nf_conntrack_max using github.com/google/cadvisor/lib
machine topology. nf_conntrack_max is host-wide, so it must be sized from the
node's CPU count, not runtime.NumCPU(): the latter honors the process cpuset
and undercounts when kube-proxy runs under a static CPU policy, which is the
behavior cadvisor worked around (kubernetes/kubernetes#99225).
Use cpuset.NumCPU() from k8s.io/utils, which reads the node's online CPU count
from /sys/devices/system/cpu/online, with a runtime.NumCPU() fallback. This
drops the cadvisor dependency from pkg/proxy/conntrack and bumps k8s.io/utils
to pick up cpuset.NumCPU.
Add import-boss .import-restrictions so the lean github.com/google/cadvisor/lib module is allowed only in the directories that consume it -- pkg/kubelet, cmd/kubelet/app, and pkg/proxy/conntrack -- and forbidden everywhere else. The repo-root .import-restrictions default-denies all github.com/google/cadvisor imports; the three consumer subtrees re-allow only .../lib.
Migrate the kubelet (and the kube-proxy conntrack helper) off the full github.com/google/cadvisor module onto the lean github.com/google/cadvisor/lib: repoint info/v1+info/v2 type usage to lib/model and the manager/fs/cache/etc. consumers to lib/*; regenerate the cadvisor.Interface mocks; keep the kubelet-pinned cAdvisor global flags via lib/cadvisorflags.
go.mod: require + replace github.com/google/cadvisor/lib (=> the dims/cadvisor/lib fork for now, until lib is tagged) and drop the full github.com/google/cadvisor module entirely; keep github.com/containerd/containerd/api at v1.11.0 (matching upstream master); add github.com/google/cadvisor to unwanted-dependencies.json unwantedModules so the full module cannot be re-vendored.
test/e2e_node: the one remaining consumer of the full module -- the node-e2e ResourceCollector, which used the v2 HTTP client (client/v2) + v2 API types (info/v2) -- now scrapes the standalone cAdvisor pod's /api/v2.1/stats directly over HTTP+JSON, so test/e2e_node depends on no cAdvisor package. No change to the kubelet's stats surfaces.
The conntrack reconciler skips services without serving endpoints, so
conntrack entries established while endpoints existed are never removed
when a UDP service scales down to zero. The REJECT (iptables) / reject
(nftables) rule installed for such services does not cover those flows:
they are DNATed to the deleted endpoint IP before the rule, which
matches on the service IP, can be evaluated. One-way UDP senders (e.g.
statsd clients) refresh the 30s conntrack timeout with every packet, so
the stale flows blackhole traffic to the deleted pod IP indefinitely;
recovery only happens when the service gets an endpoint again.
This was handled before the reconciler rewrite (kubernetes#127318):
the event-based cleanup cleared entries for every deleted UDP endpoint
regardless of how many endpoints remained.
Process services with an empty endpoints set instead of skipping them,
so every entry directed to their ClusterIP, LoadBalancer IP and
ExternalIP frontends is treated as stale and deleted.
NodePort cleanup is still skipped for services without serving
endpoints: NodePort entries are matched on the destination port only,
and with an empty endpoints set that would also remove UDP flows not
owned by kube-proxy (e.g. traffic to an unrelated host on the same
port).
This has been replaced by `//build:...` for a long time now.
Removal of the old build tag was automated with:
for i in $(git grep -l '^// +build' | grep -v -e '^vendor/'); do if ! grep -q '^// Code generated' "$i"; then sed -i -e '/^\/\/ +build/d' "$i"; fi; done
Previously, we created a separate filter for each stale flow,
resulting in O(n^2) complexity when deleting flows because the
netlink llibrary iterates over all filters for each flow.
This change introduces a new filter backed by a `sets.Set` for O(1) lookup per flow.
This reduces the overall complexity of cleaning up stale entries to O(n).
With this commit, the conntrack reconciler clears the stales
entries when endpoints change port without changing IP.
Signed-off-by: Daman Arora <aroradaman@gmail.com>
The conntrack reconciler maintains the consistency between the
conntrack table on each node and the desired state of Kubernetes UDP services.
A valid entry matches a service's ClusterIP, LoadBalancerIP, or ExternalIP and Service port,
or any ip matching a NodePort, and has a reverse source IP matching an active endpoint for
that service. Other entries are deleted.
Services without endpoints and traffic not handled by kube-proxy are ignored
Co-authored-by: Daman Arora <aroradaman@gmail.com>
kubeproxy_conntrack_reconciler_deleted_entries_total can be used
to track total entries deleted in conntrack reconciliation.
Signed-off-by: Daman Arora <aroradaman@gmail.com>
kube_proxy_conntrack_reconciler_sync_duration_seconds can be used
to track the latency of conntrack flow reconciliation.
Signed-off-by: Daman Arora <aroradaman@gmail.com>
kube-proxy needs to delete stale conntrack entries for UDP services to
avoid blackholing traffic. Instead of using the conntrack binary it
can use netlink calls directly, reducing the containers images size and
the security surface.
Signed-off-by: Daman Arora <aroradaman@gmail.com>
Co-authored-by: Antonio Ojea <aojea@google.com>
And use the fake interface in the unit tests, removing the dependency
on setting up FakeExec stuff when conntrack cleanup will be invoked.
Also, remove the isIPv6 argument to CleanStaleEntries, because it can
be inferred from the other args.
The iptables and nftables proxy backends had 2 unit tests
(TestDeleteEndpointConnections and TestProxierDeleteNodePortStaleUDP)
that were effectively testing that:
- If the proxy saw various Service/EndpointSlice events this would
result in specific changes to the service/endpoints trackers, AND
- If the service/endpoints trackers changed in those specific ways
this would result in specific UpdateServiceMapResult and
UpdateEndpointsMapResult values being generated, AND
- If you passed those specific UpdateServiceMapResult and
UpdateEndpointsMapResult values to conntrack.CleanStaleEntries it
would make specific calls to the lower-level conntrack methods,
AND
- If you called the lower-level conntrack methods with those
specific arguments, it would result in specific executions of the
conntrack binary, mixed with a specific number of klog
invocations.
This... is not a good unit test. We already test the change tracker
behavior in other unit tests, and we already tested the
Update{Service,Endpoints}MapResult behavior in the pkg/proxy unit
tests, and we already tested the conntrack exec behavior in
pkg/proxy/conntrack/conntrack_test.go, and we now test the
CleanStaleEntries behavior in pkg/proxy/conntrack/cleanup_test.go. So
there is no need to try to test the top-to-bottom behavior as a "unit
test".
Add an interface between CleanStaleEntries and the lower-level
conntrack helpers (ClearEntriesForIP, etc), and a fake implementation
of that interface, so that we can explicitly test CleanStaleEntries's
logic.
Remove some comments from conntrack.go that were explaining the
functions' callers rather than explaining the functions themselves
(and which were redundant with other comments in the callers anyway).
Fix the test names to match the functions they are testing.
Abstract out the repetitive FakeExec handling.
Explicitly specify the "expectCommand" in each one, to make it clearer
that that's really the part that we're testing.
For everything except TestExec(), test each case with both a "success"
result and a "nothing to delete" result from the conntrack binary.
The use of "Endpoint" vs "Endpoints" in these type names is tricky
because it doesn't always make sense to use the same singular/plural
convention as the corresonding service-related types, since often the
service-related type is referring to a single service while the
endpoint-related type is referring to multiple endpoint IPs.
The "endpointsInfo" types in the iptables and winkernel proxiers are
now "endpointInfo" because they describe a single endpoint IP (and
wrap proxy.BaseEndpointInfo).
"UpdateEndpointMapResult" is now "UpdateEndpointsMapResult", because
it is the result of EndpointsMap.Update (and it's clearly correct for
EndpointsMap to have plural "Endpoints" because it's a map to an array
of proxy.Endpoint objects.)
"EndpointChangeTracker" is now "EndpointsChangeTracker" because it
tracks changes to the full set of endpoints for a particular service
(and the new name matches the existing "endpointsChange" type and
"Proxier.endpointsChanges" fields.)
