From d9c342efdb8cc6cb98ae1b8f5645977ddc67f50f Mon Sep 17 00:00:00 2001 From: Akanksha Trehun Date: Sat, 29 Aug 2026 09:47:41 +0530 Subject: [PATCH] fix: avoid duplicate iptables NAT rule on container restart StaticNetwork.NetworkSetup() calls setNATRule() unconditionally on every setup, appending a MASQUERADE rule with -A regardless of whether it is already there. Since the network namespace survives a Kubernetes container restart (the same condition that used to leak TAP devices before CleanupAllUruncTaps() was added for #406), Kill() never removes this rule, so every restart appends another identical one to the same namespace. Check for the rule with iptables -C before appending it with -A, so setNATRule() becomes idempotent across restarts. Fixes #992 Signed-off-by: Akanksha Trehun --- pkg/network/network_static.go | 73 ++++++++++++++++++++++++++++------- 1 file changed, 58 insertions(+), 15 deletions(-) diff --git a/pkg/network/network_static.go b/pkg/network/network_static.go index fa46608cc..ce05fbb99 100644 --- a/pkg/network/network_static.go +++ b/pkg/network/network_static.go @@ -16,6 +16,7 @@ package network import ( "bytes" + "errors" "fmt" "os" "os/exec" @@ -29,11 +30,55 @@ var StaticIPAddr = fmt.Sprintf("%s/24", constants.StaticNetworkTapIP) type StaticNetwork struct { } -// Apply the following rule: +// natRuleArgs returns the common iptables arguments identifying the NAT +// rule that setNATRule applies, minus the leading action flag (e.g. "-A" +// or "-C"), so that the same filter can be used both to check for the +// rule's presence and to append it. +func natRuleArgs(iface string, sourceIP string) []string { + return []string{ + "-t", "nat", + "POSTROUTING", + "-s", sourceIP, + "-o", iface, + "-j", "MASQUERADE", + "--wait", "1", + } +} + +// natRuleExists checks whether the NAT rule identified by ruleArgs is +// already present in the POSTROUTING chain, via "iptables -C". iptables +// exits with 0 if the rule exists and with 1 if it does not. +func natRuleExists(path string, ruleArgs []string) (bool, error) { + var stdout, stderr bytes.Buffer + + args := append([]string{path, "-C"}, ruleArgs...) + cmd := exec.Cmd{ + Path: path, + Args: args, + Stdout: &stdout, + Stderr: &stderr, + } + err := cmd.Run() + if err == nil { + return true, nil + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return false, nil + } + return false, fmt.Errorf("iptables command %s failed: %s", cmd.String(), stderr.String()) +} + +// Apply the following rule, if not already present: // iptables -t nat -A POSTROUTING -o -s -j MASQUERADE --wait 1 // and write 1 to /proc/sys/net/ipv4/ip_forward to enable IP forwarding. +// +// Since the network namespace, and therefore any iptables rules in it, +// persists across container restarts in Kubernetes (see the equivalent +// TAP device leak fixed for #406), we check whether the rule already +// exists before appending it, to avoid piling up duplicate rules on +// every restart. func setNATRule(iface string, sourceIP string) error { - var args []string var stdout, stderr bytes.Buffer path, err := exec.LookPath("iptables") @@ -53,20 +98,18 @@ func setNATRule(iface string, sourceIP string) error { } netlog.Debug("Enabled IP forwarding") - args = append(args, path) - args = append(args, "-t") - args = append(args, "nat") - args = append(args, "-A") - args = append(args, "POSTROUTING") - args = append(args, "-s") - args = append(args, sourceIP) - args = append(args, "-o") - args = append(args, iface) - args = append(args, "-j") - args = append(args, "MASQUERADE") - args = append(args, "--wait") - args = append(args, "1") + ruleArgs := natRuleArgs(iface, sourceIP) + + exists, err := natRuleExists(path, ruleArgs) + if err != nil { + return err + } + if exists { + netlog.Debug("iptables NAT rule already present, skipping") + return nil + } + args := append([]string{path, "-A"}, ruleArgs...) cmd := exec.Cmd{ Path: path, Args: args,