Merge pull request #7401 from vmarmol/rkt-kill-pod

Kubelet: Move killPod() logic to DockerManager's KillPod()

pkg/kubelet/container/runtime.go

@@ -47,7 +47,7 @@ type Runtime interface {
 	// RunPod starts all the containers of a pod within a namespace.
 	RunPod(*api.Pod, map[string]volume.Volume) error
 	// KillPod kills all the containers of a pod.
-	KillPod(*api.Pod) error
+	KillPod(pod Pod) error
 	// RunContainerInPod starts a container within the same namespace of a pod.
 	RunContainerInPod(api.Container, *api.Pod, map[string]volume.Volume) error
 	// KillContainerInPod kills a container in the pod.

pkg/kubelet/dockertools/manager.go

@@ -433,6 +433,15 @@ func (dm *DockerManager) GetPodStatus(pod *api.Pod) (*api.PodStatus, error) {
 	return &podStatus, nil
 }
 
+func (dm *DockerManager) GetPodInfraContainer(pod kubecontainer.Pod) (kubecontainer.Container, error) {
+	for _, container := range pod.Containers {
+		if container.Name == PodInfraContainerName {
+			return *container, nil
+		}
+	}
+	return kubecontainer.Container{}, fmt.Errorf("unable to find pod infra container for pod %v", pod.ID)
+}
+
 func (dm *DockerManager) GetRunningContainers(ids []string) ([]*docker.Container, error) {
 	var result []*docker.Container
 	if dm.client == nil {
@@ -930,6 +939,35 @@ func (dm *DockerManager) PortForward(pod *kubecontainer.Pod, port uint16, stream
 	return command.Run()
 }
 
+// Kills all containers in the specified pod
+func (dm *DockerManager) KillPod(pod kubecontainer.Pod) error {
+	// Send the kills in parallel since they may take a long time.
+	errs := make(chan error, len(pod.Containers))
+	wg := sync.WaitGroup{}
+	for _, container := range pod.Containers {
+		wg.Add(1)
+		go func(container *kubecontainer.Container) {
+			defer util.HandleCrash()
+			err := dm.KillContainer(container.ID)
+			if err != nil {
+				glog.Errorf("Failed to delete container: %v; Skipping pod %q", err, pod.ID)
+				errs <- err
+			}
+			wg.Done()
+		}(container)
+	}
+	wg.Wait()
+	close(errs)
+	if len(errs) > 0 {
+		errList := []error{}
+		for err := range errs {
+			errList = append(errList, err)
+		}
+		return fmt.Errorf("failed to delete containers (%v)", errList)
+	}
+	return nil
+}
+
 // KillContainer kills a container identified by containerID.
 // Internally, it invokes docker's StopContainer API with a timeout of 10s.
 // TODO(yifan): Use new ContainerID type.

pkg/kubelet/kubelet.go

@@ -27,7 +27,6 @@ import (
 	"path"
 	"sort"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -867,39 +866,25 @@ func (kl *Kubelet) pullImage(img string, ref *api.ObjectReference) error {
 
 // Kill all running containers in a pod (includes the pod infra container).
 func (kl *Kubelet) killPod(pod kubecontainer.Pod) error {
-	// Send the kills in parallel since they may take a long time.
+	// TODO(vmarmol): Consider handling non-Docker runtimes, the plugins are not friendly to it today.
-	errs := make(chan error, len(pod.Containers))
+	container, err := kl.containerManager.GetPodInfraContainer(pod)
-	wg := sync.WaitGroup{}
+	errList := []error{}
-	for _, container := range pod.Containers {
+	if err == nil {
-		wg.Add(1)
+		// Call the networking plugin for teardown.
-		go func(container *kubecontainer.Container) {
+		err = kl.networkPlugin.TearDownPod(pod.Namespace, pod.Name, dockertools.DockerID(container.ID))
-			defer util.HandleCrash()
+		if err != nil {
-			// Call the networking plugin for teardown.
+			glog.Errorf("Failed tearing down the network plugin for pod %q: %v", pod.ID, err)
-			// TODO: Handle this without signaling the pod infra container to
-			// adapt to the generic container runtime.
-			if container.Name == dockertools.PodInfraContainerName {
-				err := kl.networkPlugin.TearDownPod(pod.Namespace, pod.Name, dockertools.DockerID(container.ID))
-				if err != nil {
-					glog.Errorf("Failed tearing down the infra container: %v", err)
-					errs <- err
-				}
-			}
-			err := kl.containerManager.KillContainer(container.ID)
-			if err != nil {
-				glog.Errorf("Failed to delete container: %v; Skipping pod %q", err, pod.ID)
-				errs <- err
-			}
-			wg.Done()
-		}(container)
-	}
-	wg.Wait()
-	close(errs)
-	if len(errs) > 0 {
-		errList := []error{}
-		for err := range errs {
 			errList = append(errList, err)
 		}
-		return fmt.Errorf("failed to delete containers (%v)", errList)
+	}
+
+	err = kl.containerManager.KillPod(pod)
+	if err != nil {
+		errList = append(errList, err)
+	}
+
+	if len(errList) > 0 {
+		return utilErrors.NewAggregate(errList)
 	}
 	return nil
 }