Fix CNI portmapping cleanup

e2e/portmap_test.go

@@ -0,0 +1,81 @@
+//
+// This is the e2e package to run tests for Ignite
+// Currently, we do local e2e tests only
+// we have to wait until the CI setup to allow Ignite to run with sudo and in a KVM environment.
+//
+// How to run tests:
+// sudo IGNITE_E2E_HOME=$PWD $(which go) test ./e2e/. -v -count 1 -run Test
+//
+
+package e2e
+
+import (
+	"fmt"
+	"os/exec"
+	"testing"
+
+	"github.com/weaveworks/ignite/e2e/util"
+	"gotest.tools/assert"
+)
+
+func TestPortmapCleanup(t *testing.T) {
+	assert.Assert(t, e2eHome != "", "IGNITE_E2E_HOME should be set")
+
+	vmName := "e2e-test-ignite-portmap-cleanup"
+	mappedPort := 4242
+
+	igniteCmd := util.NewCommand(t, igniteBin)
+
+	igniteCmd.New().
+		WithNetwork("cni").
+		With("run").
+		With("--name=" + vmName).
+		With("--ssh").
+		With("--ports=" + fmt.Sprintf("%d:%d", mappedPort, mappedPort)).
+		With(util.DefaultVMImage).
+		Run()
+
+	// Get the VM ID
+	idCmd := igniteCmd.New().
+		With("ps").
+		With("--filter").
+		With(fmt.Sprintf("{{.ObjectMeta.Name}}=%s", vmName)).
+		With("--quiet")
+
+	idOut, idErr := idCmd.Cmd.CombinedOutput()
+	assert.Check(t, idErr, fmt.Sprintf("vm id not found: \n%q\n%s", idCmd.Cmd, idOut))
+	vmID := string(idOut[:len(idOut)-2])
+
+	// Check that the IPtable rules are installed
+	grepOut, grepErr := grepIPTables(vmID)
+	assert.NilError(t, grepErr, fmt.Sprintf("unable to match iptable rules:\n %s", grepOut))
+
+	igniteCmd.New().
+		With("rm", "-f", vmName).
+		Run()
+
+	// Check that the IPtable rules are removed
+	grepOut, grepErr = grepIPTables(vmID)
+	assert.Error(t, grepErr, "exit status 1", fmt.Sprintf("unexpected output in grep : \n%s", grepOut))
+	assert.Equal(t, len(grepOut), 0, fmt.Sprintf("leftover iptable rules detected:\n%s", string(grepOut)))
+
+}
+
+func grepIPTables(search string) ([]byte, error) {
+	grepCmd := exec.Command(
+		"grep",
+		search,
+	)
+	natCmd := exec.Command(
+		"iptables",
+		"-t",
+		"nat",
+		"-nL",
+	)
+	pipe, _ := natCmd.StdoutPipe()
+	defer pipe.Close()
+
+	grepCmd.Stdin = pipe
+	_ = natCmd.Start()
+	return grepCmd.Output()
+}

pkg/network/cni/cni.go

@@ -189,7 +189,7 @@ func cniToIgniteResult(r *gocni.CNIResult) *network.Result {
 	return result
 }
 
-func (plugin *cniNetworkPlugin) RemoveContainerNetwork(containerID string) (err error) {
+func (plugin *cniNetworkPlugin) RemoveContainerNetwork(containerID string, portMappings ...meta.PortMapping) (err error) {
 	if err = plugin.initialize(); err != nil {
 		return err
 	}
@@ -214,7 +214,21 @@ func (plugin *cniNetworkPlugin) RemoveContainerNetwork(containerID string) (err
 		return nil
 	}
 
-	return plugin.cni.Remove(context.Background(), containerID, netnsPath)
+	pms := make([]gocni.PortMapping, 0, len(portMappings))
+	for _, pm := range portMappings {
+		hostIP := ""
+		if pm.BindAddress != nil {
+			hostIP = pm.BindAddress.String()
+		}
+		pms = append(pms, gocni.PortMapping{
+			HostPort:      int32(pm.HostPort),
+			ContainerPort: int32(pm.VMPort),
+			Protocol:      pm.Protocol.String(),
+			HostIP:        hostIP,
+		})
+	}
+
+	return plugin.cni.Remove(context.Background(), containerID, netnsPath, gocni.WithCapabilityPortMap(pms))
 }
 
 // cleanupBridges makes the defaultNetworkName CNI network config not leak iptables rules

pkg/network/docker/docker.go

@@ -44,7 +44,7 @@ func (plugin *dockerNetworkPlugin) SetupContainerNetwork(containerID string, _ .
 	}, nil
 }
 
-func (*dockerNetworkPlugin) RemoveContainerNetwork(_ string) error {
+func (*dockerNetworkPlugin) RemoveContainerNetwork(_ string, _ ...meta.PortMapping) error {
 	// no-op for docker, this is handled automatically
 	return nil
 }

pkg/network/types.go

@@ -21,7 +21,7 @@ type Plugin interface {
 	SetupContainerNetwork(containerID string, portmappings ...meta.PortMapping) (*Result, error)
 
 	// RemoveContainerNetwork is the method called before a container using the network plugin can be deleted
-	RemoveContainerNetwork(containerID string) error
+	RemoveContainerNetwork(containerID string, portmappings ...meta.PortMapping) error
 }
 
 type Result struct {

pkg/operations/remove.go

@@ -6,6 +6,7 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	api "github.com/weaveworks/ignite/pkg/apis/ignite"
+	meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
 	"github.com/weaveworks/ignite/pkg/client"
 	"github.com/weaveworks/ignite/pkg/dmlegacy/cleanup"
 	"github.com/weaveworks/ignite/pkg/logs"
@@ -86,7 +87,7 @@ func StopVM(vm *api.VM, kill, silent bool) error {
 	}
 
 	// Remove VM networking
-	if err = removeNetworking(util.NewPrefixer().Prefix(vm.GetUID())); err != nil {
+	if err = removeNetworking(util.NewPrefixer().Prefix(vm.GetUID()), vm.Spec.Network.Ports...); err != nil {
 		log.Warnf("Failed to cleanup networking for stopped container %s %q: %v", vm.GetKind(), vm.GetUID(), err)
 
 		return err
@@ -119,7 +120,7 @@ func StopVM(vm *api.VM, kill, silent bool) error {
 	return nil
 }
 
-func removeNetworking(containerID string) error {
+func removeNetworking(containerID string, portmappings ...meta.PortMapping) error {
 	log.Infof("Removing the container with ID %q from the %q network", containerID, providers.NetworkPlugin.Name())
-	return providers.NetworkPlugin.RemoveContainerNetwork(containerID)
+	return providers.NetworkPlugin.RemoveContainerNetwork(containerID, portmappings...)
 }