diff --git a/pkg/kubelet/dockertools/container_gc.go b/pkg/kubelet/dockertools/container_gc.go index fae2194b06..9d957dacdb 100644 --- a/pkg/kubelet/dockertools/container_gc.go +++ b/pkg/kubelet/dockertools/container_gc.go @@ -145,14 +145,20 @@ func (cgc *containerGC) evictableContainers(minAge time.Duration) (containersByE continue } else if data.State.Running { continue - } else if newestGCTime.Before(data.Created) { + } + + created, err := parseDockerTimestamp(data.Created) + if err != nil { + glog.Errorf("Failed to parse Created timestamp %q for container %q", data.Created, container.ID) + } + if newestGCTime.Before(created) { continue } containerInfo := containerGCInfo{ id: container.ID, name: container.Names[0], - createTime: data.Created, + createTime: created, } containerName, _, err := ParseDockerName(container.Names[0]) diff --git a/pkg/kubelet/dockertools/container_gc_test.go b/pkg/kubelet/dockertools/container_gc_test.go index d6038e26a0..36630ebed3 100644 --- a/pkg/kubelet/dockertools/container_gc_test.go +++ b/pkg/kubelet/dockertools/container_gc_test.go @@ -23,7 +23,6 @@ import ( "testing" "time" - docker "github.com/fsouza/go-dockerclient" "github.com/stretchr/testify/assert" "k8s.io/kubernetes/pkg/api" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" @@ -44,26 +43,22 @@ func makeTime(id int) time.Time { } // Makes a container with the specified properties. -func makeContainer(id, uid, name string, running bool, created time.Time) *docker.Container { - return &docker.Container{ - Name: fmt.Sprintf("/k8s_%s_bar_new_%s_42", name, uid), - State: docker.State{ - Running: running, - }, - ID: id, - Created: created, +func makeContainer(id, uid, name string, running bool, created time.Time) *FakeContainer { + return &FakeContainer{ + Name: fmt.Sprintf("/k8s_%s_bar_new_%s_42", name, uid), + Running: running, + ID: id, + CreatedAt: created, } } // Makes a container with unidentified name and specified properties. -func makeUndefinedContainer(id string, running bool, created time.Time) *docker.Container { - return &docker.Container{ - Name: "/k8s_unidentified", - State: docker.State{ - Running: running, - }, - ID: id, - Created: created, +func makeUndefinedContainer(id string, running bool, created time.Time) *FakeContainer { + return &FakeContainer{ + Name: "/k8s_unidentified", + Running: running, + ID: id, + CreatedAt: created, } } @@ -96,7 +91,7 @@ func verifyStringArrayEqualsAnyOrder(t *testing.T, actual, expected []string) { func TestGarbageCollectZeroMaxContainers(t *testing.T) { gc, fakeDocker := newTestContainerGC(t) - fakeDocker.SetFakeContainers([]*docker.Container{ + fakeDocker.SetFakeContainers([]*FakeContainer{ makeContainer("1876", "foo", "POD", false, makeTime(0)), }) addPods(gc.podGetter, "foo") @@ -107,7 +102,7 @@ func TestGarbageCollectZeroMaxContainers(t *testing.T) { func TestGarbageCollectNoMaxPerPodContainerLimit(t *testing.T) { gc, fakeDocker := newTestContainerGC(t) - fakeDocker.SetFakeContainers([]*docker.Container{ + fakeDocker.SetFakeContainers([]*FakeContainer{ makeContainer("1876", "foo", "POD", false, makeTime(0)), makeContainer("2876", "foo1", "POD", false, makeTime(1)), makeContainer("3876", "foo2", "POD", false, makeTime(2)), @@ -122,7 +117,7 @@ func TestGarbageCollectNoMaxPerPodContainerLimit(t *testing.T) { func TestGarbageCollectNoMaxLimit(t *testing.T) { gc, fakeDocker := newTestContainerGC(t) - fakeDocker.SetFakeContainers([]*docker.Container{ + fakeDocker.SetFakeContainers([]*FakeContainer{ makeContainer("1876", "foo", "POD", false, makeTime(0)), makeContainer("2876", "foo1", "POD", false, makeTime(0)), makeContainer("3876", "foo2", "POD", false, makeTime(0)), @@ -136,12 +131,12 @@ func TestGarbageCollectNoMaxLimit(t *testing.T) { func TestGarbageCollect(t *testing.T) { tests := []struct { - containers []*docker.Container + containers []*FakeContainer expectedRemoved []string }{ // Don't remove containers started recently. { - containers: []*docker.Container{ + containers: []*FakeContainer{ makeContainer("1876", "foo", "POD", false, time.Now()), makeContainer("2876", "foo", "POD", false, time.Now()), makeContainer("3876", "foo", "POD", false, time.Now()), @@ -149,7 +144,7 @@ func TestGarbageCollect(t *testing.T) { }, // Remove oldest containers. { - containers: []*docker.Container{ + containers: []*FakeContainer{ makeContainer("1876", "foo", "POD", false, makeTime(0)), makeContainer("2876", "foo", "POD", false, makeTime(1)), makeContainer("3876", "foo", "POD", false, makeTime(2)), @@ -158,7 +153,7 @@ func TestGarbageCollect(t *testing.T) { }, // Only remove non-running containers. { - containers: []*docker.Container{ + containers: []*FakeContainer{ makeContainer("1876", "foo", "POD", true, makeTime(0)), makeContainer("2876", "foo", "POD", false, makeTime(1)), makeContainer("3876", "foo", "POD", false, makeTime(2)), @@ -168,13 +163,13 @@ func TestGarbageCollect(t *testing.T) { }, // Less than maxContainerCount doesn't delete any. { - containers: []*docker.Container{ + containers: []*FakeContainer{ makeContainer("1876", "foo", "POD", false, makeTime(0)), }, }, // maxContainerCount applies per (UID,container) pair. { - containers: []*docker.Container{ + containers: []*FakeContainer{ makeContainer("1876", "foo", "POD", false, makeTime(0)), makeContainer("2876", "foo", "POD", false, makeTime(1)), makeContainer("3876", "foo", "POD", false, makeTime(2)), @@ -189,7 +184,7 @@ func TestGarbageCollect(t *testing.T) { }, // Remove non-running unidentified Kubernetes containers. { - containers: []*docker.Container{ + containers: []*FakeContainer{ makeUndefinedContainer("1876", true, makeTime(0)), makeUndefinedContainer("2876", false, makeTime(0)), makeContainer("3876", "foo", "POD", false, makeTime(0)), @@ -198,7 +193,7 @@ func TestGarbageCollect(t *testing.T) { }, // Max limit applied and tries to keep from every pod. { - containers: []*docker.Container{ + containers: []*FakeContainer{ makeContainer("1876", "foo", "POD", false, makeTime(0)), makeContainer("2876", "foo", "POD", false, makeTime(1)), makeContainer("3876", "foo1", "POD", false, makeTime(0)), @@ -214,7 +209,7 @@ func TestGarbageCollect(t *testing.T) { }, // If more pods than limit allows, evicts oldest pod. { - containers: []*docker.Container{ + containers: []*FakeContainer{ makeContainer("1876", "foo", "POD", false, makeTime(1)), makeContainer("2876", "foo", "POD", false, makeTime(2)), makeContainer("3876", "foo1", "POD", false, makeTime(1)), @@ -230,7 +225,7 @@ func TestGarbageCollect(t *testing.T) { }, // Containers for deleted pods should be GC'd. { - containers: []*docker.Container{ + containers: []*FakeContainer{ makeContainer("1876", "foo", "POD", false, makeTime(1)), makeContainer("2876", "foo", "POD", false, makeTime(2)), makeContainer("3876", "deleted", "POD", false, makeTime(1)), diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index aa898edaca..e74433f424 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -59,7 +59,7 @@ const ( // DockerInterface is an abstract interface for testability. It abstracts the interface of docker.Client. type DockerInterface interface { ListContainers(options dockertypes.ContainerListOptions) ([]dockertypes.Container, error) - InspectContainer(id string) (*docker.Container, error) + InspectContainer(id string) (*dockertypes.ContainerJSON, error) CreateContainer(docker.CreateContainerOptions) (*docker.Container, error) StartContainer(id string, hostConfig *docker.HostConfig) error StopContainer(id string, timeout uint) error diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index b07d76732e..09345ed33c 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -83,7 +83,7 @@ func findPodContainer(dockerContainers []*dockertypes.Container, podFullName str func TestGetContainerID(t *testing.T) { fakeDocker := NewFakeDockerClient() - fakeDocker.SetFakeRunningContainers([]*docker.Container{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{ { ID: "foobar", Name: "/k8s_foo_qux_ns_1234_42", diff --git a/pkg/kubelet/dockertools/exec.go b/pkg/kubelet/dockertools/exec.go index 426b5fb2e0..9b82977d23 100644 --- a/pkg/kubelet/dockertools/exec.go +++ b/pkg/kubelet/dockertools/exec.go @@ -23,6 +23,7 @@ import ( "os/exec" "time" + dockertypes "github.com/docker/engine-api/types" docker "github.com/fsouza/go-dockerclient" "github.com/golang/glog" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" @@ -30,14 +31,14 @@ import ( // ExecHandler knows how to execute a command in a running Docker container. type ExecHandler interface { - ExecInContainer(client DockerInterface, container *docker.Container, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error + ExecInContainer(client DockerInterface, container *dockertypes.ContainerJSON, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error } // NsenterExecHandler executes commands in Docker containers using nsenter. type NsenterExecHandler struct{} // TODO should we support nsenter in a container, running with elevated privs and --pid=host? -func (*NsenterExecHandler) ExecInContainer(client DockerInterface, container *docker.Container, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error { +func (*NsenterExecHandler) ExecInContainer(client DockerInterface, container *dockertypes.ContainerJSON, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error { nsenter, err := exec.LookPath("nsenter") if err != nil { return fmt.Errorf("exec unavailable - unable to locate nsenter") @@ -98,7 +99,7 @@ func (*NsenterExecHandler) ExecInContainer(client DockerInterface, container *do // NativeExecHandler executes commands in Docker containers using Docker's exec API. type NativeExecHandler struct{} -func (*NativeExecHandler) ExecInContainer(client DockerInterface, container *docker.Container, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error { +func (*NativeExecHandler) ExecInContainer(client DockerInterface, container *dockertypes.ContainerJSON, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error { createOpts := docker.CreateExecOptions{ Container: container.ID, Cmd: cmd, diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index 458ce624bb..ddc7af0e50 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/pkg/kubelet/dockertools/fake_docker_client.go @@ -27,6 +27,7 @@ import ( "time" dockertypes "github.com/docker/engine-api/types" + dockercontainer "github.com/docker/engine-api/types/container" docker "github.com/fsouza/go-dockerclient" "k8s.io/kubernetes/pkg/api" @@ -38,7 +39,7 @@ type FakeDockerClient struct { sync.Mutex RunningContainerList []dockertypes.Container ExitedContainerList []dockertypes.Container - ContainerMap map[string]*docker.Container + ContainerMap map[string]*dockertypes.ContainerJSON Image *docker.Image Images []docker.APIImages Errors map[string]error @@ -69,7 +70,7 @@ func NewFakeDockerClientWithVersion(version, apiVersion string) *FakeDockerClien VersionInfo: docker.Env{fmt.Sprintf("Version=%s", version), fmt.Sprintf("ApiVersion=%s", apiVersion)}, Errors: make(map[string]error), RemovedImages: sets.String{}, - ContainerMap: make(map[string]*docker.Container), + ContainerMap: make(map[string]*dockertypes.ContainerJSON), } } @@ -103,25 +104,64 @@ func (f *FakeDockerClient) ClearCalls() { f.Removed = []string{} } -func (f *FakeDockerClient) SetFakeContainers(containers []*docker.Container) { +// Because the new data type returned by engine-api is too complex to manually initialize, we need a +// fake container which is easier to initialize. +type FakeContainer struct { + ID string + Name string + Running bool + ExitCode int + Pid int + CreatedAt time.Time + StartedAt time.Time + FinishedAt time.Time + Config *dockercontainer.Config + HostConfig *dockercontainer.HostConfig +} + +// convertFakeContainer converts the fake container to real container +func convertFakeContainer(f *FakeContainer) *dockertypes.ContainerJSON { + if f.Config == nil { + f.Config = &dockercontainer.Config{} + } + if f.HostConfig == nil { + f.HostConfig = &dockercontainer.HostConfig{} + } + return &dockertypes.ContainerJSON{ + ContainerJSONBase: &dockertypes.ContainerJSONBase{ + ID: f.ID, + Name: f.Name, + State: &dockertypes.ContainerState{ + Running: f.Running, + ExitCode: f.ExitCode, + Pid: f.Pid, + StartedAt: dockerTimestampToString(f.StartedAt), + FinishedAt: dockerTimestampToString(f.FinishedAt), + }, + Created: dockerTimestampToString(f.CreatedAt), + HostConfig: f.HostConfig, + }, + Config: f.Config, + NetworkSettings: &dockertypes.NetworkSettings{}, + } +} + +func (f *FakeDockerClient) SetFakeContainers(containers []*FakeContainer) { f.Lock() defer f.Unlock() // Reset the lists and the map. - f.ContainerMap = map[string]*docker.Container{} + f.ContainerMap = map[string]*dockertypes.ContainerJSON{} f.RunningContainerList = []dockertypes.Container{} f.ExitedContainerList = []dockertypes.Container{} for i := range containers { c := containers[i] - if c.Config == nil { - c.Config = &docker.Config{} - } - f.ContainerMap[c.ID] = c + f.ContainerMap[c.ID] = convertFakeContainer(c) container := dockertypes.Container{ Names: []string{c.Name}, ID: c.ID, } - if c.State.Running { + if c.Running { f.RunningContainerList = append(f.RunningContainerList, container) } else { f.ExitedContainerList = append(f.ExitedContainerList, container) @@ -129,9 +169,9 @@ func (f *FakeDockerClient) SetFakeContainers(containers []*docker.Container) { } } -func (f *FakeDockerClient) SetFakeRunningContainers(containers []*docker.Container) { +func (f *FakeDockerClient) SetFakeRunningContainers(containers []*FakeContainer) { for _, c := range containers { - c.State.Running = true + c.Running = true } f.SetFakeContainers(containers) } @@ -228,7 +268,7 @@ func (f *FakeDockerClient) ListContainers(options dockertypes.ContainerListOptio // InspectContainer is a test-spy implementation of DockerInterface.InspectContainer. // It adds an entry "inspect" to the internal method call record. -func (f *FakeDockerClient) InspectContainer(id string) (*docker.Container, error) { +func (f *FakeDockerClient) InspectContainer(id string) (*dockertypes.ContainerJSON, error) { f.Lock() defer f.Unlock() f.called = append(f.called, "inspect_container") @@ -280,11 +320,17 @@ func (f *FakeDockerClient) CreateContainer(c docker.CreateContainerOptions) (*do f.RunningContainerList = append([]dockertypes.Container{ {ID: name, Names: []string{name}, Image: c.Config.Image, Labels: c.Config.Labels}, }, f.RunningContainerList...) - container := docker.Container{ID: name, Name: name, Config: c.Config, HostConfig: c.HostConfig} - containerCopy := container + // TODO(random-liu): Remove this convertion when we refactor CreateContainer + config := &dockercontainer.Config{} + convertType(c.Config, config) + hostConfig := &dockercontainer.HostConfig{} + convertType(c.HostConfig, hostConfig) + container := convertFakeContainer(&FakeContainer{ID: name, Name: name, Config: config, HostConfig: hostConfig}) + containerCopy := *container f.ContainerMap[name] = &containerCopy f.normalSleep(100, 25, 25) - return &container, nil + // TODO(random-liu): This will be refactored soon, we could do this because only the returned id is used. + return &docker.Container{ID: container.ID}, nil } // StartContainer is a test-spy implementation of DockerInterface.StartContainer. @@ -302,14 +348,12 @@ func (f *FakeDockerClient) StartContainer(id string, _ *docker.HostConfig) error } container, ok := f.ContainerMap[id] if !ok { - container = &docker.Container{ID: id, Name: id} + container = convertFakeContainer(&FakeContainer{ID: id, Name: id}) } - container.State = docker.State{ - Running: true, - Pid: os.Getpid(), - StartedAt: time.Now(), - } - container.NetworkSettings = &docker.NetworkSettings{IPAddress: "2.3.4.5"} + container.State.Running = true + container.State.Pid = os.Getpid() + container.State.StartedAt = dockerTimestampToString(time.Now()) + container.NetworkSettings.IPAddress = "2.3.4.5" f.ContainerMap[id] = container f.updateContainerStatus(id, statusRunningPrefix) f.normalSleep(200, 50, 50) @@ -340,17 +384,15 @@ func (f *FakeDockerClient) StopContainer(id string, timeout uint) error { f.RunningContainerList = newList container, ok := f.ContainerMap[id] if !ok { - container = &docker.Container{ - ID: id, - Name: id, - State: docker.State{ - Running: false, - StartedAt: time.Now().Add(-time.Second), - FinishedAt: time.Now(), - }, - } + container = convertFakeContainer(&FakeContainer{ + ID: id, + Name: id, + Running: false, + StartedAt: time.Now().Add(-time.Second), + FinishedAt: time.Now(), + }) } else { - container.State.FinishedAt = time.Now() + container.State.FinishedAt = dockerTimestampToString(time.Now()) container.State.Running = false } f.ContainerMap[id] = container diff --git a/pkg/kubelet/dockertools/instrumented_docker.go b/pkg/kubelet/dockertools/instrumented_docker.go index 66faa7572d..605ad15841 100644 --- a/pkg/kubelet/dockertools/instrumented_docker.go +++ b/pkg/kubelet/dockertools/instrumented_docker.go @@ -58,7 +58,7 @@ func (in instrumentedDockerInterface) ListContainers(options dockertypes.Contain return out, err } -func (in instrumentedDockerInterface) InspectContainer(id string) (*docker.Container, error) { +func (in instrumentedDockerInterface) InspectContainer(id string) (*dockertypes.ContainerJSON, error) { const operation = "inspect_container" defer recordOperation(operation, time.Now()) diff --git a/pkg/kubelet/dockertools/kube_docker_client.go b/pkg/kubelet/dockertools/kube_docker_client.go index daeac20faa..b77241ab30 100644 --- a/pkg/kubelet/dockertools/kube_docker_client.go +++ b/pkg/kubelet/dockertools/kube_docker_client.go @@ -20,9 +20,11 @@ import ( "bytes" "encoding/base64" "encoding/json" + "fmt" "io" "io/ioutil" "strconv" + "time" "github.com/docker/docker/pkg/stdcopy" dockerapi "github.com/docker/engine-api/client" @@ -112,20 +114,15 @@ func (k *kubeDockerClient) ListContainers(options dockertypes.ContainerListOptio return apiContainers, nil } -func (d *kubeDockerClient) InspectContainer(id string) (*docker.Container, error) { +func (d *kubeDockerClient) InspectContainer(id string) (*dockertypes.ContainerJSON, error) { containerJSON, err := d.client.ContainerInspect(getDefaultContext(), id) if err != nil { - // TODO(random-liu): Use IsErrContainerNotFound instead of NoSuchContainer error if dockerapi.IsErrContainerNotFound(err) { - err = &docker.NoSuchContainer{ID: id, Err: err} + return nil, containerNotFoundError{ID: id} } return nil, err } - container := &docker.Container{} - if err := convertType(&containerJSON, container); err != nil { - return nil, err - } - return container, nil + return &containerJSON, nil } func (d *kubeDockerClient) CreateContainer(opts docker.CreateContainerOptions) (*docker.Container, error) { @@ -380,3 +377,25 @@ func (d *kubeDockerClient) holdHijackedConnection(tty bool, inputStream io.Reade } return nil } + +// parseDockerTimestamp parses the timestamp returned by DockerInterface from string to time.Time +func parseDockerTimestamp(s string) (time.Time, error) { + // Timestamp returned by Docker is in time.RFC3339Nano format. + return time.Parse(time.RFC3339Nano, s) +} + +// dockerTimestampToString converts the timestamp to string +func dockerTimestampToString(t time.Time) string { + return t.Format(time.RFC3339Nano) +} + +// containerNotFoundError is the error returned by InspectContainer when container not found. We +// add this error type for testability. We don't use the original error returned by engine-api +// because dockertypes.containerNotFoundError is private, we can't create and inject it in our test. +type containerNotFoundError struct { + ID string +} + +func (e containerNotFoundError) Error() string { + return fmt.Sprintf("Error: No such container: %s", e.ID) +} diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 22008a262b..1f209715db 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -314,7 +314,7 @@ var ( // determineContainerIP determines the IP address of the given container. It is expected // that the container passed is the infrastructure container of a pod and the responsibility // of the caller to ensure that the correct container is passed. -func (dm *DockerManager) determineContainerIP(podNamespace, podName string, container *docker.Container) string { +func (dm *DockerManager) determineContainerIP(podNamespace, podName string, container *dockertypes.ContainerJSON) string { result := "" if container.NetworkSettings != nil { @@ -352,6 +352,20 @@ func (dm *DockerManager) inspectContainer(id string, podName, podNamespace strin var containerInfo *labelledContainerInfo containerInfo = getContainerInfoFromLabel(iResult.Config.Labels) + parseTimestampError := func(label, s string) { + glog.Errorf("Failed to parse %q timestamp %q for container %q of pod %q", label, s, id, kubecontainer.BuildPodFullName(podName, podNamespace)) + } + var createdAt, startedAt, finishedAt time.Time + if createdAt, err = parseDockerTimestamp(iResult.Created); err != nil { + parseTimestampError("Created", iResult.Created) + } + if startedAt, err = parseDockerTimestamp(iResult.State.StartedAt); err != nil { + parseTimestampError("StartedAt", iResult.State.StartedAt) + } + if finishedAt, err = parseDockerTimestamp(iResult.State.FinishedAt); err != nil { + parseTimestampError("FinishedAt", iResult.State.FinishedAt) + } + status := kubecontainer.ContainerStatus{ Name: containerName, RestartCount: containerInfo.RestartCount, @@ -359,13 +373,13 @@ func (dm *DockerManager) inspectContainer(id string, podName, podNamespace strin ImageID: DockerPrefix + iResult.Image, ID: kubecontainer.DockerID(id).ContainerID(), ExitCode: iResult.State.ExitCode, - CreatedAt: iResult.Created, + CreatedAt: createdAt, Hash: hash, } if iResult.State.Running { // Container that are running, restarting and paused status.State = kubecontainer.ContainerStateRunning - status.StartedAt = iResult.State.StartedAt + status.StartedAt = startedAt if containerName == PodInfraContainerName { ip = dm.determineContainerIP(podNamespace, podName, iResult) } @@ -373,13 +387,11 @@ func (dm *DockerManager) inspectContainer(id string, podName, podNamespace strin } // Find containers that have exited or failed to start. - if !iResult.State.FinishedAt.IsZero() || iResult.State.ExitCode != 0 { + if !finishedAt.IsZero() || iResult.State.ExitCode != 0 { // Containers that are exited, dead or created (docker failed to start container) // When a container fails to start State.ExitCode is non-zero, FinishedAt and StartedAt are both zero reason := "" message := iResult.State.Error - finishedAt := iResult.State.FinishedAt - startedAt := iResult.State.StartedAt // Note: An application might handle OOMKilled gracefully. // In that case, the container is oom killed, but the exit @@ -388,14 +400,14 @@ func (dm *DockerManager) inspectContainer(id string, podName, podNamespace strin reason = "OOMKilled" } else if iResult.State.ExitCode == 0 { reason = "Completed" - } else if !iResult.State.FinishedAt.IsZero() { + } else if !finishedAt.IsZero() { reason = "Error" } else { // finishedAt is zero and ExitCode is nonZero occurs when docker fails to start the container reason = ErrContainerCannotRun.Error() // Adjust time to the time docker attempted to run the container, otherwise startedAt and finishedAt will be set to epoch, which is misleading - finishedAt = iResult.Created - startedAt = iResult.Created + finishedAt = createdAt + startedAt = createdAt } terminationMessagePath := containerInfo.TerminationMessagePath @@ -865,9 +877,9 @@ func readOnlyRootFilesystem(container *api.Container) bool { } // container must not be nil -func getDockerNetworkMode(container *docker.Container) string { +func getDockerNetworkMode(container *dockertypes.ContainerJSON) string { if container.HostConfig != nil { - return container.HostConfig.NetworkMode + return string(container.HostConfig.NetworkMode) } return "" } @@ -1405,7 +1417,7 @@ func (dm *DockerManager) killContainer(containerID kubecontainer.ContainerID, co var errNoPodOnContainer = fmt.Errorf("no pod information labels on Docker container") // containerAndPodFromLabels tries to load the appropriate container info off of a Docker container's labels -func containerAndPodFromLabels(inspect *docker.Container) (pod *api.Pod, container *api.Container, err error) { +func containerAndPodFromLabels(inspect *dockertypes.ContainerJSON) (pod *api.Pod, container *api.Container, err error) { if inspect == nil && inspect.Config == nil && inspect.Config.Labels == nil { return nil, nil, errNoPodOnContainer } @@ -1444,7 +1456,7 @@ func containerAndPodFromLabels(inspect *docker.Container) (pod *api.Pod, contain return } -func (dm *DockerManager) applyOOMScoreAdj(container *api.Container, containerInfo *docker.Container) error { +func (dm *DockerManager) applyOOMScoreAdj(container *api.Container, containerInfo *dockertypes.ContainerJSON) error { cgroupName, err := dm.procFs.GetFullContainerName(containerInfo.State.Pid) if err != nil { if err == os.ErrNotExist { @@ -1550,7 +1562,7 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe return id, err } -func (dm *DockerManager) applyOOMScoreAdjIfNeeded(container *api.Container, containerInfo *docker.Container) error { +func (dm *DockerManager) applyOOMScoreAdjIfNeeded(container *api.Container, containerInfo *dockertypes.ContainerJSON) error { // Compare current API version with expected api version. result, err := dm.checkDockerAPIVersion(dockerv110APIVersion) if err != nil { @@ -1929,8 +1941,7 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, _ api.PodStatus, podStatus *kubec } // Setup the host interface unless the pod is on the host's network (FIXME: move to networkPlugin when ready) - var podInfraContainer *docker.Container - podInfraContainer, err = dm.client.InspectContainer(string(podInfraContainerID)) + podInfraContainer, err := dm.client.InspectContainer(string(podInfraContainerID)) if err != nil { glog.Errorf("Failed to inspect pod infra container: %v; Skipping pod %q", err, format.Pod(pod)) result.Fail(err) @@ -2172,7 +2183,7 @@ func (dm *DockerManager) GetPodStatus(uid types.UID, name, namespace string) (*k } result, ip, err := dm.inspectContainer(c.ID, name, namespace) if err != nil { - if _, ok := err.(*docker.NoSuchContainer); ok { + if _, ok := err.(containerNotFoundError); ok { // https://github.com/kubernetes/kubernetes/issues/22541 // Sometimes when docker's state is corrupt, a container can be listed // but couldn't be inspected. We fake a status for this container so diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index c0a7c73f6e..2d18c8bbe2 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/pkg/kubelet/dockertools/manager_test.go @@ -30,6 +30,7 @@ import ( "time" dockertypes "github.com/docker/engine-api/types" + dockercontainer "github.com/docker/engine-api/types/container" docker "github.com/fsouza/go-dockerclient" cadvisorapi "github.com/google/cadvisor/info/v1" "github.com/stretchr/testify/assert" @@ -320,7 +321,7 @@ func verifyPods(a, b []*kubecontainer.Pod) bool { func TestGetPods(t *testing.T) { manager, fakeDocker := newTestDockerManager() - dockerContainers := []*docker.Container{ + dockerContainers := []*FakeContainer{ { ID: "1111", Name: "/k8s_foo_qux_new_1234_42", @@ -406,7 +407,7 @@ func TestKillContainerInPod(t *testing.T) { }, Spec: api.PodSpec{Containers: []api.Container{{Name: "foo"}, {Name: "bar"}}}, } - containers := []*docker.Container{ + containers := []*FakeContainer{ { ID: "1111", Name: "/k8s_foo_qux_new_1234_42", @@ -465,11 +466,11 @@ func TestKillContainerInPodWithPreStop(t *testing.T) { if err != nil { t.Errorf("unexpected error: %v", err) } - containers := []*docker.Container{ + containers := []*FakeContainer{ { ID: "1111", Name: "/k8s_foo_qux_new_1234_42", - Config: &docker.Config{ + Config: &dockercontainer.Config{ Labels: map[string]string{ kubernetesPodLabel: string(podString), kubernetesContainerNameLabel: "foo", @@ -508,7 +509,7 @@ func TestKillContainerInPodWithError(t *testing.T) { }, Spec: api.PodSpec{Containers: []api.Container{{Name: "foo"}, {Name: "bar"}}}, } - containers := []*docker.Container{ + containers := []*FakeContainer{ { ID: "1111", Name: "/k8s_foo_qux_new_1234_42", @@ -675,7 +676,7 @@ func TestSyncPodWithPodInfraCreatesContainer(t *testing.T) { }, } - fakeDocker.SetFakeRunningContainers([]*docker.Container{{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{{ ID: "9876", // Pod infra container. Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_0", @@ -709,7 +710,7 @@ func TestSyncPodDeletesWithNoPodInfraContainer(t *testing.T) { }, }, } - fakeDocker.SetFakeRunningContainers([]*docker.Container{{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{{ ID: "1234", Name: "/k8s_bar1_foo1_new_12345678_0", }}) @@ -752,7 +753,7 @@ func TestSyncPodDeletesDuplicate(t *testing.T) { }, } - fakeDocker.SetFakeRunningContainers([]*docker.Container{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{ { ID: "1234", Name: "/k8s_foo_bar_new_12345678_1111", @@ -793,7 +794,7 @@ func TestSyncPodBadHash(t *testing.T) { }, } - fakeDocker.SetFakeRunningContainers([]*docker.Container{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{ { ID: "1234", Name: "/k8s_bar.1234_foo_new_12345678_42", @@ -831,7 +832,7 @@ func TestSyncPodsUnhealthy(t *testing.T) { }, } - fakeDocker.SetFakeRunningContainers([]*docker.Container{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{ { ID: unhealthyContainerID, Name: "/k8s_unhealthy_foo_new_12345678_42", @@ -871,7 +872,7 @@ func TestSyncPodsDoesNothing(t *testing.T) { }, }, } - fakeDocker.SetFakeRunningContainers([]*docker.Container{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{ { ID: "1234", Name: "/k8s_bar." + strconv.FormatUint(kubecontainer.HashContainer(&container), 16) + "_foo_new_12345678_0", @@ -902,35 +903,26 @@ func TestSyncPodWithRestartPolicy(t *testing.T) { Containers: containers, }, } - dockerContainers := []*docker.Container{ + dockerContainers := []*FakeContainer{ { - ID: "9876", - Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_0", - Config: &docker.Config{}, - State: docker.State{ - StartedAt: time.Now(), - Running: true, - }, + ID: "9876", + Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_0", + StartedAt: time.Now(), + Running: true, }, { - ID: "1234", - Name: "/k8s_succeeded." + strconv.FormatUint(kubecontainer.HashContainer(&containers[0]), 16) + "_foo_new_12345678_0", - Config: &docker.Config{}, - State: docker.State{ - ExitCode: 0, - StartedAt: time.Now(), - FinishedAt: time.Now(), - }, + ID: "1234", + Name: "/k8s_succeeded." + strconv.FormatUint(kubecontainer.HashContainer(&containers[0]), 16) + "_foo_new_12345678_0", + ExitCode: 0, + StartedAt: time.Now(), + FinishedAt: time.Now(), }, { - ID: "5678", - Name: "/k8s_failed." + strconv.FormatUint(kubecontainer.HashContainer(&containers[1]), 16) + "_foo_new_12345678_0", - Config: &docker.Config{}, - State: docker.State{ - ExitCode: 42, - StartedAt: time.Now(), - FinishedAt: time.Now(), - }, + ID: "5678", + Name: "/k8s_failed." + strconv.FormatUint(kubecontainer.HashContainer(&containers[1]), 16) + "_foo_new_12345678_0", + ExitCode: 42, + StartedAt: time.Now(), + FinishedAt: time.Now(), }} tests := []struct { @@ -1007,31 +999,25 @@ func TestSyncPodBackoff(t *testing.T) { } stableId := "k8s_bad." + strconv.FormatUint(kubecontainer.HashContainer(&containers[1]), 16) + "_podfoo_nsnew_12345678" - dockerContainers := []*docker.Container{ + dockerContainers := []*FakeContainer{ { - ID: "9876", - Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_podfoo_nsnew_12345678_0", - State: docker.State{ - StartedAt: startTime, - Running: true, - }, + ID: "9876", + Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_podfoo_nsnew_12345678_0", + StartedAt: startTime, + Running: true, }, { - ID: "1234", - Name: "/k8s_good." + strconv.FormatUint(kubecontainer.HashContainer(&containers[0]), 16) + "_podfoo_nsnew_12345678_0", - State: docker.State{ - StartedAt: startTime, - Running: true, - }, + ID: "1234", + Name: "/k8s_good." + strconv.FormatUint(kubecontainer.HashContainer(&containers[0]), 16) + "_podfoo_nsnew_12345678_0", + StartedAt: startTime, + Running: true, }, { - ID: "5678", - Name: "/k8s_bad." + strconv.FormatUint(kubecontainer.HashContainer(&containers[1]), 16) + "_podfoo_nsnew_12345678_0", - State: docker.State{ - ExitCode: 42, - StartedAt: startTime, - FinishedAt: fakeClock.Now(), - }, + ID: "5678", + Name: "/k8s_bad." + strconv.FormatUint(kubecontainer.HashContainer(&containers[1]), 16) + "_podfoo_nsnew_12345678_0", + ExitCode: 42, + StartedAt: startTime, + FinishedAt: fakeClock.Now(), }, } @@ -1080,7 +1066,7 @@ func TestSyncPodBackoff(t *testing.T) { if len(fakeDocker.Created) > 0 { // pretend kill the container fakeDocker.Created = nil - dockerContainers[2].State.FinishedAt = startTime.Add(time.Duration(c.killDelay) * time.Second) + dockerContainers[2].FinishedAt = startTime.Add(time.Duration(c.killDelay) * time.Second) } } } @@ -1200,7 +1186,7 @@ func TestGetTerminationMessagePath(t *testing.T) { // One for infra container, one for container "bar" t.Fatalf("unexpected container list length %d", len(containerList)) } - inspectResult, err := dm.client.InspectContainer(containerList[0].ID) + inspectResult, err := fakeDocker.InspectContainer(containerList[0].ID) if err != nil { t.Fatalf("unexpected inspect error: %v", err) } @@ -1238,7 +1224,7 @@ func TestSyncPodWithPodInfraCreatesContainerCallsHandler(t *testing.T) { }, }, } - fakeDocker.SetFakeRunningContainers([]*docker.Container{{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{{ ID: "9876", Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_0", }}) @@ -1288,7 +1274,7 @@ func TestSyncPodEventHandlerFails(t *testing.T) { }, } - fakeDocker.SetFakeRunningContainers([]*docker.Container{{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{{ ID: "9876", Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_0", }}) @@ -1694,7 +1680,7 @@ func TestSyncPodWithFailure(t *testing.T) { puller.HasImages = []string{test.container.Image} // Pretend that the pod infra container has already been created, so that // we can run the user containers. - fakeDocker.SetFakeRunningContainers([]*docker.Container{{ + fakeDocker.SetFakeRunningContainers([]*FakeContainer{{ ID: "9876", Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_0", }}) @@ -1878,29 +1864,25 @@ func TestGetPodStatusNoSuchContainer(t *testing.T) { }, } - fakeDocker.SetFakeContainers([]*docker.Container{ + fakeDocker.SetFakeContainers([]*FakeContainer{ { - ID: noSuchContainerID, - Name: "/k8s_nosuchcontainer_foo_new_12345678_42", - State: docker.State{ - ExitCode: 0, - StartedAt: time.Now(), - FinishedAt: time.Now(), - Running: false, - }, + ID: noSuchContainerID, + Name: "/k8s_nosuchcontainer_foo_new_12345678_42", + ExitCode: 0, + StartedAt: time.Now(), + FinishedAt: time.Now(), + Running: false, }, { - ID: infraContainerID, - Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_42", - State: docker.State{ - ExitCode: 0, - StartedAt: time.Now(), - FinishedAt: time.Now(), - Running: false, - }, - }}) - - fakeDocker.InjectErrors(map[string]error{"inspect": &docker.NoSuchContainer{}}) + ID: infraContainerID, + Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_42", + ExitCode: 0, + StartedAt: time.Now(), + FinishedAt: time.Now(), + Running: false, + }, + }) + fakeDocker.InjectErrors(map[string]error{"inspect_container": containerNotFoundError{}}) runSyncPod(t, dm, fakeDocker, pod, nil, false) // Verify that we will try to start new contrainers even if the inspections diff --git a/pkg/kubelet/network/cni/cni_test.go b/pkg/kubelet/network/cni/cni_test.go index 3d2179a0d9..dbf237d46e 100644 --- a/pkg/kubelet/network/cni/cni_test.go +++ b/pkg/kubelet/network/cni/cni_test.go @@ -30,7 +30,6 @@ import ( clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" - docker "github.com/fsouza/go-dockerclient" cadvisorapi "github.com/google/cadvisor/info/v1" "k8s.io/kubernetes/cmd/kubelet/app/options" @@ -132,10 +131,10 @@ func (fnh *fakeNetworkHost) GetKubeClient() clientset.Interface { func (nh *fakeNetworkHost) GetRuntime() kubecontainer.Runtime { dm, fakeDockerClient := newTestDockerManager() - fakeDockerClient.SetFakeRunningContainers([]*docker.Container{ + fakeDockerClient.SetFakeRunningContainers([]*dockertools.FakeContainer{ { - ID: "test_infra_container", - State: docker.State{Pid: 12345}, + ID: "test_infra_container", + Pid: 12345, }, }) return dm