Merge pull request #691 from networkop/portmap-fix

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...)
 }