diff --git a/cmd/nerdctl/container/container_remove_linux_test.go b/cmd/nerdctl/container/container_remove_linux_test.go index cda9aa1b8b9..57ea219fb44 100644 --- a/cmd/nerdctl/container/container_remove_linux_test.go +++ b/cmd/nerdctl/container/container_remove_linux_test.go @@ -23,6 +23,8 @@ import ( "testing" "time" + cniutils "github.com/containernetworking/plugins/pkg/utils" + "github.com/containerd/nerdctl/mod/tigron/expect" "github.com/containerd/nerdctl/mod/tigron/require" "github.com/containerd/nerdctl/mod/tigron/test" @@ -111,10 +113,15 @@ func TestContainerRmIptables(t *testing.T) { Expected: func(data test.Data, helpers test.Helpers) *test.Expected { // Get the container ID from the label containerID := data.Labels().Get("containerID") + id := fmt.Sprintf("%s-%s", testutil.Namespace, containerID) + chain := cniutils.FormatChainName("bridge", id) return &test.Expected{ ExitCode: expect.ExitCodeSuccess, - // Verify that the iptables output does not contain the container ID - Output: expect.DoesNotContain(containerID), + // Verify that the iptables output does not contain the container ID and a chain name generated from the cni name, namespace, and container ID + Output: expect.All( + expect.DoesNotContain(containerID), + expect.DoesNotContain(chain), + ), } }, }, diff --git a/go.mod b/go.mod index f656e9d7c94..d7208299a56 100644 --- a/go.mod +++ b/go.mod @@ -152,6 +152,7 @@ require ( github.com/moby/moby/client v0.1.0 // indirect github.com/moby/sys/capability v0.4.0 // indirect go.yaml.in/yaml/v4 v4.0.0-rc.4 // indirect + sigs.k8s.io/knftables v0.0.18 // indirect ) replace github.com/containerd/nerdctl/mod/tigron v0.0.0 => ./mod/tigron diff --git a/go.sum b/go.sum index be8cbd3516a..17f46d99a36 100644 --- a/go.sum +++ b/go.sum @@ -187,6 +187,8 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/lithammer/dedent v1.1.0 h1:VNzHMVCBNG1j0fh3OrsFRkVUwStdDArbgBWoPAffktY= +github.com/lithammer/dedent v1.1.0/go.mod h1:jrXYCQtgg0nJiN+StA2KgR7w6CiQNv9Fd/Z9BP0jIOc= github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= github.com/mattn/go-isatty v0.0.21 h1:xYae+lCNBP7QuW4PUnNG61ffM4hVIfm+zUzDuSzYLGs= @@ -516,6 +518,8 @@ lukechampine.com/blake3 v1.3.0 h1:sJ3XhFINmHSrYCgl958hscfIa3bw8x4DqMP3u1YvoYE= lukechampine.com/blake3 v1.3.0/go.mod h1:0OFRp7fBtAylGVCO40o87sbupkyIGgbpv1+M1k1LM6k= pgregory.net/rapid v1.2.0 h1:keKAYRcjm+e1F0oAuU5F5+YPAWcyxNNRK2wud503Gnk= pgregory.net/rapid v1.2.0/go.mod h1:PY5XlDGj0+V1FCq0o192FdRhpKHGTRIWBgqjDBTrq04= +sigs.k8s.io/knftables v0.0.18 h1:6Duvmu0s/HwGifKrtl6G3AyAPYlWiZqTgS8bkVMiyaE= +sigs.k8s.io/knftables v0.0.18/go.mod h1:f/5ZLKYEUPUhVjUCg6l80ACdL7CIIyeL0DxfgojGRTk= sigs.k8s.io/yaml v1.6.0 h1:G8fkbMSAFqgEFgh4b1wmtzDnioxFCUgTZhlbj5P9QYs= sigs.k8s.io/yaml v1.6.0/go.mod h1:796bPqUfzR/0jLAl6XjHl3Ck7MiyVv8dbTdyT3/pMf4= tags.cncf.io/container-device-interface v1.1.0 h1:RnxNhxF1JOu6CJUVpetTYvrXHdxw9j9jFYgZpI+anSY= diff --git a/pkg/ocihook/ocihook.go b/pkg/ocihook/ocihook.go index 0522949cfea..e8b4579da80 100644 --- a/pkg/ocihook/ocihook.go +++ b/pkg/ocihook/ocihook.go @@ -722,7 +722,7 @@ func onPostStop(opts *handlerOpts) error { // opts.cni.Remove has trouble removing network configurations when netns is empty. // Therefore, we force the deletion of iptables rules here to prevent netns exhaustion. // This is a workaround until https://github.com/containernetworking/plugins/pull/1078 is merged. - if err := cleanupIptablesRules(opts.fullID); err != nil { + if err := cleanupIptablesRules(opts.fullID, opts.cniNames); err != nil { log.L.WithError(err).Warnf("failed to clean up iptables rules for container %s", opts.fullID) // Don't return error here, continue with the rest of the cleanup } @@ -755,43 +755,6 @@ func onPostStop(opts *handlerOpts) error { return nil } -// cleanupIptablesRules cleans up iptables rules related to the container -func cleanupIptablesRules(containerID string) error { - // Check if iptables command exists - if _, err := exec.LookPath("iptables"); err != nil { - return fmt.Errorf("iptables command not found: %w", err) - } - - // Tables to check for rules - tables := []string{"nat", "filter", "mangle"} - - for _, table := range tables { - // Get all iptables rules for this table - cmd := exec.Command("iptables", "-t", table, "-S") - output, err := cmd.CombinedOutput() - if err != nil { - log.L.WithError(err).Warnf("failed to list iptables rules for table %s", table) - continue - } - - // Find and delete rules related to the container - rules := strings.Split(string(output), "\n") - for _, rule := range rules { - if strings.Contains(rule, containerID) { - // Execute delete command - deleteCmd := exec.Command("sh", "-c", "--", fmt.Sprintf(`iptables -t %s -D %s`, table, rule[3:])) - if deleteOutput, err := deleteCmd.CombinedOutput(); err != nil { - log.L.WithError(err).Warnf("failed to delete iptables rule: %s, output: %s", rule, string(deleteOutput)) - } else { - log.L.Debugf("deleted iptables rule: %s", rule) - } - } - } - } - - return nil -} - // writePidFile writes the pid atomically to a file. // From https://github.com/containerd/containerd/blob/v1.7.0-rc.2/cmd/ctr/commands/commands.go#L265-L282 func writePidFile(path string, pid int) error { diff --git a/pkg/ocihook/ocihook_linux.go b/pkg/ocihook/ocihook_linux.go index da961d75435..bb3bd29cb3c 100644 --- a/pkg/ocihook/ocihook_linux.go +++ b/pkg/ocihook/ocihook_linux.go @@ -17,6 +17,12 @@ package ocihook import ( + "fmt" + "os/exec" + "strings" + + cniutils "github.com/containernetworking/plugins/pkg/utils" + "github.com/containerd/containerd/v2/contrib/apparmor" "github.com/containerd/log" @@ -37,3 +43,51 @@ func loadAppArmor() { // but the profile was not actually loaded, runc will fail. } } + +// cleanupIptablesRules cleans up iptables rules related to the container +func cleanupIptablesRules(containerID string, cniNames []string) error { + // Check if iptables command exists + if _, err := exec.LookPath("iptables"); err != nil { + return fmt.Errorf("iptables command not found: %w", err) + } + + // Tables to check for rules + tables := []string{"nat", "filter", "mangle"} + + for _, table := range tables { + // Get all iptables rules for this table + cmd := exec.Command("iptables", "-t", table, "-S") + output, err := cmd.CombinedOutput() + if err != nil { + log.L.WithError(err).Warnf("failed to list iptables rules for table %s", table) + continue + } + + // Find and delete rules related to the container + rules := strings.Split(string(output), "\n") + for _, rule := range rules { + if strings.Contains(rule, containerID) { + // Execute delete command + deleteCmd := exec.Command("sh", "-c", "--", fmt.Sprintf(`iptables -t %s -D %s`, table, rule[3:])) + if deleteOutput, err := deleteCmd.CombinedOutput(); err != nil { + log.L.WithError(err).Warnf("failed to delete iptables rule: %s, output: %s", rule, string(deleteOutput)) + } else { + log.L.Debugf("deleted iptables rule: %s", rule) + } + } + } + } + + // Delete CNI chains related to the container + for _, cniName := range cniNames { + chain := cniutils.FormatChainName(cniName, containerID) + deleteCmd := exec.Command("iptables", "-t", "nat", "-X", chain) + if deleteOutput, err := deleteCmd.CombinedOutput(); err != nil { + log.L.WithError(err).Warnf("failed to delete iptables chain: %s, output: %s", chain, string(deleteOutput)) + } else { + log.L.Debugf("deleted iptables chain: %s", chain) + } + } + + return nil +} diff --git a/pkg/ocihook/ocihook_nolinux.go b/pkg/ocihook/ocihook_nolinux.go index 85f465f392f..0d106383951 100644 --- a/pkg/ocihook/ocihook_nolinux.go +++ b/pkg/ocihook/ocihook_nolinux.go @@ -21,3 +21,8 @@ package ocihook func loadAppArmor() { //noop } + +func cleanupIptablesRules(containerID string, cniNames []string) error { + //noop + return nil +}