Merge pull request #9346 from yifan-gu/cleanup_kubelet_tests

pkg/kubelet/container/runtime.go

@@ -19,6 +19,7 @@ package container
 import (
 	"fmt"
 	"io"
+	"reflect"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -282,6 +283,11 @@ func (p *Pod) FindContainerByName(containerName string) *Container {
 	return nil
 }
 
+// IsEmpty returns true if the pod is empty.
+func (p *Pod) IsEmpty() bool {
+	return reflect.DeepEqual(p, &Pod{})
+}
+
 // GetPodFullName returns a name that uniquely identifies a pod.
 func GetPodFullName(pod *api.Pod) string {
 	// Use underscore as the delimiter because it is not allowed in pod name

pkg/kubelet/container_gc_test.go

@@ -18,6 +18,8 @@ package kubelet
 
 import (
 	"fmt"
+	"reflect"
+	"sort"
 	"testing"
 	"time"
 
@@ -73,6 +75,20 @@ func makeContainerDetailMap(funcs ...func(map[string]*docker.Container)) map[str
 	return m
 }
 
+func verifyStringArrayEqualsAnyOrder(t *testing.T, actual, expected []string) {
+	act := make([]string, len(actual))
+	exp := make([]string, len(expected))
+	copy(act, actual)
+	copy(exp, expected)
+
+	sort.StringSlice(act).Sort()
+	sort.StringSlice(exp).Sort()
+
+	if !reflect.DeepEqual(exp, act) {
+		t.Errorf("Expected(sorted): %#v, Actual(sorted): %#v", exp, act)
+	}
+}
+
 func TestGarbageCollectZeroMaxContainers(t *testing.T) {
 	gc, fakeDocker := newTestContainerGC(t, time.Minute, 1, 0)
 	fakeDocker.ContainerList = []docker.APIContainers{

pkg/kubelet/dockertools/manager.go

@@ -1006,6 +1006,10 @@ func (dm *DockerManager) ExecInContainer(containerId string, cmd []string, stdin
 	return dm.execHandler.ExecInContainer(dm.client, container, cmd, stdin, stdout, stderr, tty)
 }
 
+func noPodInfraContainerError(podName, podNamespace string) error {
+	return fmt.Errorf("cannot find pod infra container in pod %q", kubecontainer.BuildPodFullName(podName, podNamespace))
+}
+
 // PortForward executes socat in the pod's network namespace and copies
 // data between stream (representing the user's local connection on their
 // computer) and the specified port in the container.
@@ -1017,7 +1021,7 @@ func (dm *DockerManager) ExecInContainer(containerId string, cmd []string, stdin
 func (dm *DockerManager) PortForward(pod *kubecontainer.Pod, port uint16, stream io.ReadWriteCloser) error {
 	podInfraContainer := pod.FindContainerByName(PodInfraContainerName)
 	if podInfraContainer == nil {
-		return fmt.Errorf("cannot find pod infra container in pod %q", kubecontainer.BuildPodFullName(pod.Name, pod.Namespace))
+		return noPodInfraContainerError(pod.Name, pod.Namespace)
 	}
 	container, err := dm.client.InspectContainer(string(podInfraContainer.ID))
 	if err != nil {

pkg/kubelet/dockertools/manager_test.go

@@ -19,7 +19,9 @@ package dockertools
 import (
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"net/http"
+	"os"
 	"reflect"
 	"regexp"
 	"sort"
@@ -84,8 +86,19 @@ type fakeOptionGenerator struct{}
 
 var _ kubecontainer.RunContainerOptionsGenerator = &fakeOptionGenerator{}
 
+var testPodContainerDir string
+
 func (*fakeOptionGenerator) GenerateRunContainerOptions(pod *api.Pod, container *api.Container) (*kubecontainer.RunContainerOptions, error) {
-	return &kubecontainer.RunContainerOptions{}, nil
+	var opts kubecontainer.RunContainerOptions
+	var err error
+	if len(container.TerminationMessagePath) != 0 {
+		testPodContainerDir, err = ioutil.TempDir("", "fooPodContainerDir")
+		if err != nil {
+			return nil, err
+		}
+		opts.PodContainerDir = testPodContainerDir
+	}
+	return &opts, nil
 }
 
 func newTestDockerManagerWithHTTPClient(fakeHTTPClient *fakeHTTP) (*DockerManager, *FakeDockerClient) {
@@ -1914,3 +1927,68 @@ func TestSyncPodEventHandlerFails(t *testing.T) {
 		t.Errorf("Wrong stopped container, expected: bar, get: %q", dockerName.ContainerName)
 	}
 }
+
+func TestPortForwardNoSuchContainer(t *testing.T) {
+	dm, _ := newTestDockerManager()
+
+	podName, podNamespace := "podName", "podNamespace"
+	err := dm.PortForward(
+		&kubecontainer.Pod{
+			ID:         "podID",
+			Name:       podName,
+			Namespace:  podNamespace,
+			Containers: nil,
+		},
+		5000,
+		nil,
+	)
+	if err == nil {
+		t.Fatal("unexpected non-error")
+	}
+	expectedErr := noPodInfraContainerError(podName, podNamespace)
+	if !reflect.DeepEqual(err, expectedErr) {
+		t.Fatalf("expected %v, but saw %v", expectedErr, err)
+	}
+}
+
+func TestSyncPodWithTerminationLog(t *testing.T) {
+	dm, fakeDocker := newTestDockerManager()
+	container := api.Container{
+		Name: "bar",
+		TerminationMessagePath: "/dev/somepath",
+	}
+	fakeDocker.ContainerList = []docker.APIContainers{}
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			UID:       "12345678",
+			Name:      "foo",
+			Namespace: "new",
+		},
+		Spec: api.PodSpec{
+			Containers: []api.Container{
+				container,
+			},
+		},
+	}
+
+	runSyncPod(t, dm, fakeDocker, pod)
+	verifyCalls(t, fakeDocker, []string{
+		// Create pod infra container.
+		"create", "start", "inspect_container",
+		// Create container.
+		"create", "start", "inspect_container",
+	})
+
+	defer os.Remove(testPodContainerDir)
+
+	fakeDocker.Lock()
+	defer fakeDocker.Unlock()
+
+	parts := strings.Split(fakeDocker.Container.HostConfig.Binds[0], ":")
+	if !matchString(t, testPodContainerDir+"/k8s_bar\\.[a-f0-9]", parts[0]) {
+		t.Errorf("Unexpected host path: %s", parts[0])
+	}
+	if parts[1] != "/dev/somepath" {
+		t.Errorf("Unexpected container path: %s", parts[1])
+	}
+}

pkg/kubelet/kubelet.go

@@ -2235,6 +2235,9 @@ func (kl *Kubelet) PortForward(podFullName string, podUID types.UID, port uint16
 		return err
 	}
 	pod := kubecontainer.Pods(pods).FindPod(podFullName, podUID)
+	if pod.IsEmpty() {
+		return fmt.Errorf("pod not found (%q)", podFullName)
+	}
 	return kl.runner.PortForward(&pod, port, stream)
 }