github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-12-08 02:15:27 +00:00
Code Issues Projects Releases Wiki Activity

Remove redundant iptables/nftables conntrack cleanup tests

Browse Source 
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".
This commit is contained in:
Dan Winship
2024-01-14 09:06:35 -05:00
parent db12cbe2ae
commit cdf934d5bc
3 changed files with 6 additions and 688 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
pkg/proxy/conntrack/conntrack.go
View File

@@ -56,8 +56,8 @@ type execCT struct {
var _ Interface = &execCT{}
// NoConnectionToDelete is the error string returned by conntrack when no matching connections are found
const NoConnectionToDelete = "0 flow entries have been deleted"
// noConnectionToDelete is the error string returned by conntrack when no matching connections are found
const noConnectionToDelete = "0 flow entries have been deleted"
func protoStr(proto v1.Protocol) string {
return strings.ToLower(string(proto))
@@ -89,7 +89,7 @@ func (ct *execCT) exec(parameters ...string) error {
func (ct *execCT) ClearEntriesForIP(ip string, protocol v1.Protocol) error {
parameters := parametersWithFamily(utilnet.IsIPv6String(ip), "-D", "--orig-dst", ip, "-p", protoStr(protocol))
err := ct.exec(parameters...)
if err != nil && !strings.Contains(err.Error(), NoConnectionToDelete) {
if err != nil && !strings.Contains(err.Error(), noConnectionToDelete) {
// TODO: Better handling for deletion failure. When failure occur, stale udp connection may not get flushed.
// These stale udp connection will keep black hole traffic. Making this a best effort operation for now, since it
// is expensive to baby-sit all udp connections to kubernetes services.
@@ -105,7 +105,7 @@ func (ct *execCT) ClearEntriesForPort(port int, isIPv6 bool, protocol v1.Protoco
}
parameters := parametersWithFamily(isIPv6, "-D", "-p", protoStr(protocol), "--dport", strconv.Itoa(port))
err := ct.exec(parameters...)
if err != nil && !strings.Contains(err.Error(), NoConnectionToDelete) {
if err != nil && !strings.Contains(err.Error(), noConnectionToDelete) {
return fmt.Errorf("error deleting conntrack entries for UDP port: %d, error: %v", port, err)
}
return nil
@@ -116,7 +116,7 @@ func (ct *execCT) ClearEntriesForNAT(origin, dest string, protocol v1.Protocol)
parameters := parametersWithFamily(utilnet.IsIPv6String(origin), "-D", "--orig-dst", origin, "--dst-nat", dest,
"-p", protoStr(protocol))
err := ct.exec(parameters...)
if err != nil && !strings.Contains(err.Error(), NoConnectionToDelete) {
if err != nil && !strings.Contains(err.Error(), noConnectionToDelete) {
// TODO: Better handling for deletion failure. When failure occur, stale udp connection may not get flushed.
// These stale udp connection will keep black hole traffic. Making this a best effort operation for now, since it
// is expensive to baby sit all udp connections to kubernetes services.
@@ -132,7 +132,7 @@ func (ct *execCT) ClearEntriesForPortNAT(dest string, port int, protocol v1.Prot
}
parameters := parametersWithFamily(utilnet.IsIPv6String(dest), "-D", "-p", protoStr(protocol), "--dport", strconv.Itoa(port), "--dst-nat", dest)
err := ct.exec(parameters...)
if err != nil && !strings.Contains(err.Error(), NoConnectionToDelete) {
if err != nil && !strings.Contains(err.Error(), noConnectionToDelete) {
return fmt.Errorf("error deleting conntrack entries for UDP port: %d, error: %v", port, err)
}
return nil