Merge pull request #884 from dcbw/clean-up-sandbox

server: clean up sandbox network when running the sandbox fails

server/sandbox_network.go

@@ -0,0 +1,70 @@
+package server
+
+import (
+	"fmt"
+	"net"
+
+	"github.com/kubernetes-incubator/cri-o/libkpod/sandbox"
+	"github.com/sirupsen/logrus"
+	"k8s.io/kubernetes/pkg/kubelet/network/hostport"
+)
+
+// networkStart sets up the sandbox's network and returns the pod IP on success
+// or an error
+func (s *Server) networkStart(hostNetwork bool, sb *sandbox.Sandbox) (string, error) {
+	if hostNetwork {
+		return s.BindAddress(), nil
+	}
+
+	podNetwork := newPodNetwork(sb.Namespace(), sb.KubeName(), sb.ID(), sb.NetNsPath())
+	err := s.netPlugin.SetUpPod(podNetwork)
+	if err != nil {
+		return "", fmt.Errorf("failed to create pod network sandbox %s(%s): %v", sb.Name(), sb.ID(), err)
+	}
+
+	var ip string
+	if ip, err = s.netPlugin.GetPodNetworkStatus(podNetwork); err != nil {
+		return "", fmt.Errorf("failed to get network status for pod sandbox %s(%s): %v", sb.Name(), sb.ID(), err)
+	}
+
+	if len(sb.PortMappings()) > 0 {
+		ip4 := net.ParseIP(ip).To4()
+		if ip4 == nil {
+			return "", fmt.Errorf("failed to get valid ipv4 address for sandbox %s(%s)", sb.Name(), sb.ID())
+		}
+
+		if err = s.hostportManager.Add(sb.ID(), &hostport.PodPortMapping{
+			Name:         sb.Name(),
+			PortMappings: sb.PortMappings(),
+			IP:           ip4,
+			HostNetwork:  false,
+		}, "lo"); err != nil {
+			return "", fmt.Errorf("failed to add hostport mapping for sandbox %s(%s): %v", sb.Name(), sb.ID(), err)
+		}
+
+	}
+	return ip, nil
+}
+
+// networkStop cleans up and removes a pod's network.  It is best-effort and
+// must call the network plugin even if the network namespace is already gone
+func (s *Server) networkStop(hostNetwork bool, sb *sandbox.Sandbox) error {
+	if !hostNetwork {
+		if err := s.hostportManager.Remove(sb.ID(), &hostport.PodPortMapping{
+			Name:         sb.Name(),
+			PortMappings: sb.PortMappings(),
+			HostNetwork:  false,
+		}); err != nil {
+			logrus.Warnf("failed to remove hostport for pod sandbox %s(%s): %v",
+				sb.Name(), sb.ID(), err)
+		}
+
+		podNetwork := newPodNetwork(sb.Namespace(), sb.KubeName(), sb.ID(), sb.NetNsPath())
+		if err := s.netPlugin.TearDownPod(podNetwork); err != nil {
+			logrus.Warnf("failed to destroy network for pod sandbox %s(%s): %v",
+				sb.Name(), sb.ID(), err)
+		}
+	}
+
+	return nil
+}

server/sandbox_run.go

@@ -3,7 +3,6 @@ package server
 import (
 	"encoding/json"
 	"fmt"
-	"net"
 	"os"
 	"path"
 	"path/filepath"
@@ -98,7 +97,7 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 	defer s.updateLock.RUnlock()
 
 	logrus.Debugf("RunPodSandboxRequest %+v", req)
-	var processLabel, mountLabel, netNsPath, resolvPath string
+	var processLabel, mountLabel, resolvPath string
 	// process req.Name
 	kubeName := req.GetConfig().GetMetadata().Name
 	if kubeName == "" {
@@ -408,11 +407,6 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 		if err != nil {
 			return nil, err
 		}
-
-		netNsPath, err = sandbox.HostNetNsPath()
-		if err != nil {
-			return nil, err
-		}
 	} else {
 		// Create the sandbox network namespace
 		if err = sb.NetNsCreate(); err != nil {
@@ -434,8 +428,6 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 		if err != nil {
 			return nil, err
 		}
-
-		netNsPath = sb.NetNsPath()
 	}
 
 	if req.GetConfig().GetLinux().GetSecurityContext().GetNamespaceOptions().HostPid {
@@ -473,36 +465,15 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 	sb.SetInfraContainer(container)
 
 	var ip string
-	// setup the network
+	ip, err = s.networkStart(hostNetwork, sb)
-	if !hostNetwork {
+	if err != nil {
-		podNetwork := newPodNetwork(sb.Namespace(), sb.KubeName(), sb.ID(), netNsPath)
+		return nil, err
-		if err = s.netPlugin.SetUpPod(podNetwork); err != nil {
-			return nil, fmt.Errorf("failed to create pod network sandbox %s(%s): %v", sb.Name(), id, err)
-		}
-
-		if ip, err = s.netPlugin.GetPodNetworkStatus(podNetwork); err != nil {
-			return nil, fmt.Errorf("failed to get network status for pod sandbox %s(%s): %v", sb.Name(), id, err)
-		}
-
-		if len(portMappings) != 0 {
-			ip4 := net.ParseIP(ip).To4()
-			if ip4 == nil {
-				return nil, fmt.Errorf("failed to get valid ipv4 address for sandbox %s(%s)", sb.Name(), id)
-			}
-
-			if err = s.hostportManager.Add(id, &hostport.PodPortMapping{
-				Name:         sb.Name(),
-				PortMappings: portMappings,
-				IP:           ip4,
-				HostNetwork:  false,
-			}, "lo"); err != nil {
-				return nil, fmt.Errorf("failed to add hostport mapping for sandbox %s(%s): %v", sb.Name(), id, err)
-			}
-
-		}
-	} else {
-		ip = s.BindAddress()
 	}
+	defer func() {
+		if err != nil {
+			s.networkStop(hostNetwork, sb)
+		}
+	}()
 
 	g.AddAnnotation(annotations.IP, ip)
 	sb.AddIP(ip)

server/sandbox_stop.go

@@ -2,7 +2,6 @@ package server
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/containers/storage"
 	"github.com/docker/docker/pkg/mount"
@@ -15,7 +14,6 @@ import (
 	"golang.org/x/net/context"
 	"golang.org/x/sys/unix"
 	pb "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"
-	"k8s.io/kubernetes/pkg/kubelet/network/hostport"
 )
 
 // StopPodSandbox stops the sandbox. If there are any running containers in the
@@ -44,36 +42,14 @@ func (s *Server) StopPodSandbox(ctx context.Context, req *pb.StopPodSandboxReque
 		return resp, nil
 	}
 
-	podInfraContainer := sb.InfraContainer()
+	// Clean up sandbox networking and close its network namespace.
-	netnsPath, err := podInfraContainer.NetNsPath()
+	hostNetwork := sb.NetNsPath() == ""
-	if err != nil {
+	s.networkStop(hostNetwork, sb)
-		return nil, err
-	}
-	if _, err := os.Stat(netnsPath); err == nil {
-		if err2 := s.hostportManager.Remove(sb.ID(), &hostport.PodPortMapping{
-			Name:         sb.Name(),
-			PortMappings: sb.PortMappings(),
-			HostNetwork:  false,
-		}); err2 != nil {
-			logrus.Warnf("failed to remove hostport for pod sandbox %s(%s): %v",
-				podInfraContainer.Name(), sb.ID(), err2)
-		}
-
-		podNetwork := newPodNetwork(sb.Namespace(), sb.KubeName(), sb.ID(), netnsPath)
-		if err2 := s.netPlugin.TearDownPod(podNetwork); err2 != nil {
-			logrus.Warnf("failed to destroy network for pod sandbox %s(%s): %v",
-				sb.Name(), sb.ID(), err2)
-		}
-	} else if !os.IsNotExist(err) { // it's ok for netnsPath to *not* exist
-		return nil, fmt.Errorf("failed to stat netns path for pod sandbox %s(%s) before tearing down the network: %v",
-			sb.Name(), sb.ID(), err)
-	}
-
-	// Close the sandbox networking namespace.
 	if err := sb.NetNsRemove(); err != nil {
 		return nil, err
 	}
 
+	podInfraContainer := sb.InfraContainer()
 	containers := sb.Containers().List()
 	containers = append(containers, podInfraContainer)