fix: add network cleanup for kill and stop cmd

cmd/nerdctl/container_kill_linux_test.go

@@ -0,0 +1,78 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package main
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
+	"github.com/containerd/nerdctl/v2/pkg/testutil"
+	iptablesutil "github.com/containerd/nerdctl/v2/pkg/testutil/iptables"
+	"github.com/coreos/go-iptables/iptables"
+	"gotest.tools/v3/assert"
+)
+
+// TestKillCleanupForwards runs a container that exposes a port and then kill it.
+// The test checks that the kill command effectively clean up
+// the iptables forwards creted from the run.
+func TestKillCleanupForwards(t *testing.T) {
+	const (
+		hostPort          = 9999
+		testContainerName = "ngx"
+	)
+	base := testutil.NewBase(t)
+	defer func() {
+		base.Cmd("rm", "-f", testContainerName).Run()
+	}()
+
+	// skip if rootless
+	if rootlessutil.IsRootless() {
+		t.Skip("pkg/testutil/iptables does not support rootless")
+	}
+
+	ipt, err := iptables.New()
+	assert.NilError(t, err)
+
+	containerID := base.Cmd("run", "-d",
+		"--restart=no",
+		"--name", testContainerName,
+		"-p", fmt.Sprintf("127.0.0.1:%d:80", hostPort),
+		testutil.NginxAlpineImage).Run().Stdout()
+	containerID = strings.TrimSuffix(containerID, "\n")
+
+	containerIP := base.Cmd("inspect",
+		"-f",
+		"'{{range.NetworkSettings.Networks}}{{.IPAddress}}{{end}}'",
+		testContainerName).Run().Stdout()
+	containerIP = strings.ReplaceAll(containerIP, "'", "")
+	containerIP = strings.TrimSuffix(containerIP, "\n")
+
+	// define iptables chain name depending on the target (docker/nerdctl)
+	var chain string
+	if testutil.GetTarget() == testutil.Docker {
+		chain = "DOCKER"
+	} else {
+		redirectChain := "CNI-HOSTPORT-DNAT"
+		chain = iptablesutil.GetRedirectedChain(t, ipt, redirectChain, testutil.Namespace, containerID)
+	}
+	assert.Equal(t, iptablesutil.ForwardExists(t, ipt, chain, containerIP, hostPort), true)
+
+	base.Cmd("kill", testContainerName).AssertOK()
+	assert.Equal(t, iptablesutil.ForwardExists(t, ipt, chain, containerIP, hostPort), false)
+}

cmd/nerdctl/container_stop_linux_test.go

@@ -22,8 +22,11 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
 	"github.com/containerd/nerdctl/v2/pkg/testutil"
+	iptablesutil "github.com/containerd/nerdctl/v2/pkg/testutil/iptables"
 	"github.com/containerd/nerdctl/v2/pkg/testutil/nettestutil"
+	"github.com/coreos/go-iptables/iptables"
 
 	"gotest.tools/v3/assert"
 )
@@ -86,3 +89,49 @@ echo "signal quit"`).AssertOK()
 	base.Cmd("stop", testContainerName).AssertOK()
 	base.Cmd("logs", "-f", testContainerName).AssertOutContains("signal quit")
 }
+
+func TestStopCleanupForwards(t *testing.T) {
+	const (
+		hostPort          = 9999
+		testContainerName = "ngx"
+	)
+	base := testutil.NewBase(t)
+	defer func() {
+		base.Cmd("rm", "-f", testContainerName).Run()
+	}()
+
+	// skip if rootless
+	if rootlessutil.IsRootless() {
+		t.Skip("pkg/testutil/iptables does not support rootless")
+	}
+
+	ipt, err := iptables.New()
+	assert.NilError(t, err)
+
+	containerID := base.Cmd("run", "-d",
+		"--restart=no",
+		"--name", testContainerName,
+		"-p", fmt.Sprintf("127.0.0.1:%d:80", hostPort),
+		testutil.NginxAlpineImage).Run().Stdout()
+	containerID = strings.TrimSuffix(containerID, "\n")
+
+	containerIP := base.Cmd("inspect",
+		"-f",
+		"'{{range.NetworkSettings.Networks}}{{.IPAddress}}{{end}}'",
+		testContainerName).Run().Stdout()
+	containerIP = strings.ReplaceAll(containerIP, "'", "")
+	containerIP = strings.TrimSuffix(containerIP, "\n")
+
+	// define iptables chain name depending on the target (docker/nerdctl)
+	var chain string
+	if testutil.GetTarget() == testutil.Docker {
+		chain = "DOCKER"
+	} else {
+		redirectChain := "CNI-HOSTPORT-DNAT"
+		chain = iptablesutil.GetRedirectedChain(t, ipt, redirectChain, testutil.Namespace, containerID)
+	}
+	assert.Equal(t, iptablesutil.ForwardExists(t, ipt, chain, containerIP, hostPort), true)
+
+	base.Cmd("stop", testContainerName).AssertOK()
+	assert.Equal(t, iptablesutil.ForwardExists(t, ipt, chain, containerIP, hostPort), false)
+}

pkg/cmd/container/create.go

@@ -484,6 +484,12 @@ func withNerdctlOCIHook(cmd string, args []string) (oci.SpecOpts, error) {
 			Args: crArgs,
 			Env:  os.Environ(),
 		})
+		scArgs := append(args, "startContainer")
+		s.Hooks.CreateRuntime = append(s.Hooks.StartContainer, specs.Hook{
+			Path: cmd,
+			Args: scArgs,
+			Env:  os.Environ(),
+		})
 		argsCopy := append([]string(nil), args...)
 		psArgs := append(argsCopy, "postStop")
 		s.Hooks.Poststop = append(s.Hooks.Poststop, specs.Hook{

pkg/cmd/container/kill.go

@@ -18,6 +18,7 @@ package container
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 	"strings"
@@ -26,10 +27,16 @@ import (
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/cio"
 	"github.com/containerd/containerd/errdefs"
+	gocni "github.com/containerd/go-cni"
 	"github.com/containerd/log"
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/containerd/nerdctl/v2/pkg/containerutil"
 	"github.com/containerd/nerdctl/v2/pkg/idutil/containerwalker"
+	"github.com/containerd/nerdctl/v2/pkg/labels"
+	"github.com/containerd/nerdctl/v2/pkg/netutil"
+	"github.com/containerd/nerdctl/v2/pkg/netutil/nettype"
+	"github.com/containerd/nerdctl/v2/pkg/portutil"
+	"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
 	"github.com/moby/sys/signal"
 )
 
@@ -50,6 +57,9 @@ func Kill(ctx context.Context, client *containerd.Client, reqs []string, options
 			if found.MatchCount > 1 {
 				return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
 			}
+			if err := cleanupNetwork(ctx, found.Container, options.GOptions); err != nil {
+				return fmt.Errorf("unable to cleanup network for container: %s, %q", found.Req, err)
+			}
 			if err := killContainer(ctx, found.Container, parsedSignal); err != nil {
 				if errdefs.IsNotFound(err) {
 					fmt.Fprintf(options.Stderr, "No such container: %s\n", found.Req)
@@ -106,3 +116,77 @@ func killContainer(ctx context.Context, container containerd.Container, signal s
 	}
 	return nil
 }
+
+// cleanupNetwork removes cni network setup, specifically the forwards
+func cleanupNetwork(ctx context.Context, container containerd.Container, globalOpts types.GlobalCommandOptions) error {
+	return rootlessutil.WithDetachedNetNSIfAny(func() error {
+		// retrieve info to get current active port mappings
+		info, err := container.Info(ctx, containerd.WithoutRefreshedMetadata)
+		if err != nil {
+			return err
+		}
+		ports, portErr := portutil.ParsePortsLabel(info.Labels)
+		if portErr != nil {
+			return fmt.Errorf("no oci spec: %q", portErr)
+		}
+		portMappings := []gocni.NamespaceOpts{
+			gocni.WithCapabilityPortMap(ports),
+		}
+
+		// retrieve info to get cni instance
+		spec, err := container.Spec(ctx)
+		if err != nil {
+			return err
+		}
+		networksJSON := spec.Annotations[labels.Networks]
+		var networks []string
+		if err := json.Unmarshal([]byte(networksJSON), &networks); err != nil {
+			return err
+		}
+		netType, err := nettype.Detect(networks)
+		if err != nil {
+			return err
+		}
+
+		switch netType {
+		case nettype.Host, nettype.None, nettype.Container:
+			// NOP
+		case nettype.CNI:
+			e, err := netutil.NewCNIEnv(globalOpts.CNIPath, globalOpts.CNINetConfPath, netutil.WithDefaultNetwork())
+			if err != nil {
+				return err
+			}
+			cniOpts := []gocni.Opt{
+				gocni.WithPluginDir([]string{globalOpts.CNIPath}),
+			}
+			netMap, err := e.NetworkMap()
+			if err != nil {
+				return err
+			}
+			for _, netstr := range networks {
+				net, ok := netMap[netstr]
+				if !ok {
+					return fmt.Errorf("no such network: %q", netstr)
+				}
+				cniOpts = append(cniOpts, gocni.WithConfListBytes(net.Bytes))
+			}
+			cni, err := gocni.New(cniOpts...)
+			if err != nil {
+				return err
+			}
+
+			var namespaceOpts []gocni.NamespaceOpts
+			namespaceOpts = append(namespaceOpts, portMappings...)
+			namespace := spec.Annotations[labels.Namespace]
+			fullID := namespace + "-" + container.ID()
+			if err := cni.Remove(ctx, fullID, "", namespaceOpts...); err != nil {
+				log.L.WithError(err).Errorf("failed to call cni.Remove")
+				return err
+			}
+			return nil
+		default:
+			return fmt.Errorf("unexpected network type %v", netType)
+		}
+		return nil
+	})
+}

pkg/cmd/container/stop.go

@@ -35,6 +35,9 @@ func Stop(ctx context.Context, client *containerd.Client, reqs []string, opt typ
 			if found.MatchCount > 1 {
 				return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
 			}
+			if err := cleanupNetwork(ctx, found.Container, opt.GOptions); err != nil {
+				return fmt.Errorf("unable to cleanup network for container: %s", found.Req)
+			}
 			if err := containerutil.Stop(ctx, found.Container, opt.Timeout); err != nil {
 				if errdefs.IsNotFound(err) {
 					fmt.Fprintf(opt.Stderr, "No such container: %s\n", found.Req)