Skip to content

Commit 8fbf810

Browse files
committed
fix: clean up unused iptables chains not being deleted on container removal
When publishing a container's port(s) to the host and removeing the container, there are some iptables chains that are not deleted, as shown below: ```bash $ sudo nerdctl run -d --name nginx -p 8080:80 nginx 81cc6b08527975ef8bf151b1460ead6a3d767310a7513d306a4bbe19f61fe6a8 $ ID=$(echo -n "bridgedefault-$(sudo nerdctl ps -q --no-trunc --filter=name=nginx)" | sha512sum | awk '{print substr($1, 1, 24)}') $ sudo iptables -t nat -S | grep $ID -N CNI-5e9207ffbe238a4b386cd5bd -A POSTROUTING -s 10.4.0.156/32 -m comment --comment "name: \"bridge\" id: \"default-81cc6b08527975ef8bf151b1460ead6a3d767310a7513d306a4bbe19f61fe6a8\"" -j CNI-5e9207ffbe238a4b386cd5bd -A CNI-5e9207ffbe238a4b386cd5bd -d 10.4.0.0/24 -m comment --comment "name: \"bridge\" id: \"default-81cc6b08527975ef8bf151b1460ead6a3d767310a7513d306a4bbe19f61fe6a8\"" -j ACCEPT -A CNI-5e9207ffbe238a4b386cd5bd ! -d 224.0.0.0/4 -m comment --comment "name: \"bridge\" id: \"default-81cc6b08527975ef8bf151b1460ead6a3d767310a7513d306a4bbe19f61fe6a8\"" -j MASQUERADE $ sudo nerdctl rm -f nginx nginx $ sudo iptables -t nat -S | grep $ID -N CNI-5e9207ffbe238a4b386cd5bd $ sudo iptables -L -nv -t nat | grep $ID -3 Chain CNI-5cd4851e431cb9d7ef1a143b (0 references) pkts bytes target prot opt in out source destination Chain CNI-5e9207ffbe238a4b386cd5bd (0 references) pkts bytes target prot opt in out source destination Chain CNI-5fa88ae608b5a4cfbe76c33d (0 references) ``` Unused iptables chains should be deleted. Therefore, this PR makes a change so that the relevant iptables chains are deleted when a container is removed. Signed-off-by: Hayato Kiwata <dev@haytok.jp>
1 parent 7e75010 commit 8fbf810

File tree

4 files changed

+28
-4
lines changed

4 files changed

+28
-4
lines changed

cmd/nerdctl/container/container_remove_linux_test.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
"github.com/containerd/nerdctl/mod/tigron/require"
2828
"github.com/containerd/nerdctl/mod/tigron/test"
2929

30+
cniutils "github.com/containernetworking/plugins/pkg/utils"
31+
3032
"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
3133
"github.com/containerd/nerdctl/v2/pkg/testutil"
3234
"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
@@ -111,10 +113,15 @@ func TestContainerRmIptables(t *testing.T) {
111113
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
112114
// Get the container ID from the label
113115
containerID := data.Labels().Get("containerID")
116+
id := fmt.Sprintf("%s-%s", testutil.Namespace, containerID)
117+
chain := cniutils.FormatChainName("bridge", id)
114118
return &test.Expected{
115119
ExitCode: expect.ExitCodeSuccess,
116-
// Verify that the iptables output does not contain the container ID
117-
Output: expect.DoesNotContain(containerID),
120+
// Verify that the iptables output does not contain the container ID and a chain name generated from the cni name, namespace, and container ID
121+
Output: expect.All(
122+
expect.DoesNotContain(containerID),
123+
expect.DoesNotContain(chain),
124+
),
118125
}
119126
},
120127
},

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ require (
154154
github.com/moby/moby/client v0.1.0 // indirect
155155
github.com/moby/sys/capability v0.4.0 // indirect
156156
go.yaml.in/yaml/v4 v4.0.0-rc.3 // indirect
157+
sigs.k8s.io/knftables v0.0.18 // indirect
157158
)
158159

159160
replace github.com/containerd/nerdctl/mod/tigron v0.0.0 => ./mod/tigron

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,8 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
185185
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
186186
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
187187
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
188+
github.com/lithammer/dedent v1.1.0 h1:VNzHMVCBNG1j0fh3OrsFRkVUwStdDArbgBWoPAffktY=
189+
github.com/lithammer/dedent v1.1.0/go.mod h1:jrXYCQtgg0nJiN+StA2KgR7w6CiQNv9Fd/Z9BP0jIOc=
188190
github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE=
189191
github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8=
190192
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
@@ -515,6 +517,8 @@ lukechampine.com/blake3 v1.3.0 h1:sJ3XhFINmHSrYCgl958hscfIa3bw8x4DqMP3u1YvoYE=
515517
lukechampine.com/blake3 v1.3.0/go.mod h1:0OFRp7fBtAylGVCO40o87sbupkyIGgbpv1+M1k1LM6k=
516518
pgregory.net/rapid v1.2.0 h1:keKAYRcjm+e1F0oAuU5F5+YPAWcyxNNRK2wud503Gnk=
517519
pgregory.net/rapid v1.2.0/go.mod h1:PY5XlDGj0+V1FCq0o192FdRhpKHGTRIWBgqjDBTrq04=
520+
sigs.k8s.io/knftables v0.0.18 h1:6Duvmu0s/HwGifKrtl6G3AyAPYlWiZqTgS8bkVMiyaE=
521+
sigs.k8s.io/knftables v0.0.18/go.mod h1:f/5ZLKYEUPUhVjUCg6l80ACdL7CIIyeL0DxfgojGRTk=
518522
sigs.k8s.io/yaml v1.6.0 h1:G8fkbMSAFqgEFgh4b1wmtzDnioxFCUgTZhlbj5P9QYs=
519523
sigs.k8s.io/yaml v1.6.0/go.mod h1:796bPqUfzR/0jLAl6XjHl3Ck7MiyVv8dbTdyT3/pMf4=
520524
tags.cncf.io/container-device-interface v1.1.0 h1:RnxNhxF1JOu6CJUVpetTYvrXHdxw9j9jFYgZpI+anSY=

pkg/ocihook/ocihook.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"time"
3232

3333
types100 "github.com/containernetworking/cni/pkg/types/100"
34+
cniutils "github.com/containernetworking/plugins/pkg/utils"
3435
"github.com/opencontainers/runtime-spec/specs-go"
3536
b4nndclient "github.com/rootless-containers/bypass4netns/pkg/api/daemon/client"
3637
rlkclient "github.com/rootless-containers/rootlesskit/v2/pkg/api/client"
@@ -722,7 +723,7 @@ func onPostStop(opts *handlerOpts) error {
722723
// opts.cni.Remove has trouble removing network configurations when netns is empty.
723724
// Therefore, we force the deletion of iptables rules here to prevent netns exhaustion.
724725
// This is a workaround until https://github.com/containernetworking/plugins/pull/1078 is merged.
725-
if err := cleanupIptablesRules(opts.fullID); err != nil {
726+
if err := cleanupIptablesRules(opts.fullID, opts.cniNames); err != nil {
726727
log.L.WithError(err).Warnf("failed to clean up iptables rules for container %s", opts.fullID)
727728
// Don't return error here, continue with the rest of the cleanup
728729
}
@@ -756,7 +757,7 @@ func onPostStop(opts *handlerOpts) error {
756757
}
757758

758759
// cleanupIptablesRules cleans up iptables rules related to the container
759-
func cleanupIptablesRules(containerID string) error {
760+
func cleanupIptablesRules(containerID string, cniNames []string) error {
760761
// Check if iptables command exists
761762
if _, err := exec.LookPath("iptables"); err != nil {
762763
return fmt.Errorf("iptables command not found: %w", err)
@@ -789,6 +790,17 @@ func cleanupIptablesRules(containerID string) error {
789790
}
790791
}
791792

793+
// Delete CNI chains related to the container
794+
for _, cniName := range cniNames {
795+
chain := cniutils.FormatChainName(cniName, containerID)
796+
deleteCmd := exec.Command("iptables", "-t", "nat", "-X", chain)
797+
if deleteOutput, err := deleteCmd.CombinedOutput(); err != nil {
798+
log.L.WithError(err).Warnf("failed to delete iptables chain: %s, output: %s", chain, string(deleteOutput))
799+
} else {
800+
log.L.Debugf("deleted iptables chain: %s", chain)
801+
}
802+
}
803+
792804
return nil
793805
}
794806

0 commit comments

Comments
 (0)