Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Kubelet: Refactor container related functions in DockerInterface #23699

Merged
merged 4 commits into from
Apr 17, 2016
Merged

Kubelet: Refactor container related functions in DockerInterface #23699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b348e7d
Refactr ListContainers.
Random-Liu Apr 4, 2016
da07fa9
Refactor InspectContainer.
Random-Liu Apr 4, 2016
ba4a5ed
Refactor CreateContainer.
Random-Liu Apr 14, 2016
d3d98b3
Refactor StartContainer, StopContainer and RemoveContainer.
Random-Liu Apr 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions contrib/mesos/pkg/executor/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (

clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"

"github.com/fsouza/go-dockerclient"
dockertypes "github.com/docker/engine-api/types"
"github.com/gogo/protobuf/proto"
log "github.com/golang/glog"
bindings "github.com/mesos/mesos-go/executor"
Expand Down Expand Up @@ -655,14 +655,13 @@ func (k *Executor) doShutdown(driver bindings.ExecutorDriver) {
// Destroy existing k8s containers
func (k *Executor) killKubeletContainers() {
if containers, err := dockertools.GetKubeletDockerContainers(k.dockerClient, true); err == nil {
opts := docker.RemoveContainerOptions{
opts := dockertypes.ContainerRemoveOptions{
RemoveVolumes: true,
Force: true,
}
for _, container := range containers {
opts.ID = container.ID
log.V(2).Infof("Removing container: %v", opts.ID)
if err := k.dockerClient.RemoveContainer(opts); err != nil {
log.V(2).Infof("Removing container: %v", container.ID)
if err := k.dockerClient.RemoveContainer(container.ID, opts); err != nil {
log.Warning(err)
}
}
Expand Down
16 changes: 11 additions & 5 deletions pkg/kubelet/dockertools/container_gc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"sort"
"time"

docker "github.com/fsouza/go-dockerclient"
dockertypes "github.com/docker/engine-api/types"
"github.com/golang/glog"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
"k8s.io/kubernetes/pkg/types"
Expand Down Expand Up @@ -111,7 +111,7 @@ func (cgc *containerGC) removeOldestN(containers []containerGCInfo, toRemove int
// Remove from oldest to newest (last to first).
numToKeep := len(containers) - toRemove
for i := numToKeep; i < len(containers); i++ {
err := cgc.client.RemoveContainer(docker.RemoveContainerOptions{ID: containers[i].id, RemoveVolumes: true})
err := cgc.client.RemoveContainer(containers[i].id, dockertypes.ContainerRemoveOptions{RemoveVolumes: true})
if err != nil {
glog.Warningf("Failed to remove dead container %q: %v", containers[i].name, err)
}
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -189,7 +195,7 @@ func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy)
// Remove unidentified containers.
for _, container := range unidentifiedContainers {
glog.Infof("Removing unidentified dead container %q with ID %q", container.name, container.id)
err = cgc.client.RemoveContainer(docker.RemoveContainerOptions{ID: container.id, RemoveVolumes: true})
err = cgc.client.RemoveContainer(container.id, dockertypes.ContainerRemoveOptions{RemoveVolumes: true})
if err != nil {
glog.Warningf("Failed to remove unidentified dead container %q: %v", container.name, err)
}
Expand Down
55 changes: 25 additions & 30 deletions pkg/kubelet/dockertools/container_gc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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")
Expand All @@ -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)),
Expand All @@ -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)),
Expand All @@ -136,20 +131,20 @@ 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()),
},
},
// 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)),
Expand All @@ -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)),
Expand All @@ -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)),
Expand All @@ -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)),
Expand All @@ -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)),
Expand All @@ -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)),
Expand All @@ -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)),
Expand Down
7 changes: 4 additions & 3 deletions pkg/kubelet/dockertools/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"strings"

dockertypes "github.com/docker/engine-api/types"
docker "github.com/fsouza/go-dockerclient"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit lgtm

Expand All @@ -32,7 +33,7 @@ const (
)

func mapState(state string) kubecontainer.ContainerState {
// Parse the state string in docker.APIContainers. This could break when
// Parse the state string in dockertypes.Container. This could break when
// we upgrade docker.
switch {
case strings.HasPrefix(state, statusRunningPrefix):
Expand All @@ -44,8 +45,8 @@ func mapState(state string) kubecontainer.ContainerState {
}
}

// Converts docker.APIContainers to kubecontainer.Container.
func toRuntimeContainer(c *docker.APIContainers) (*kubecontainer.Container, error) {
// Converts dockertypes.Container to kubecontainer.Container.
func toRuntimeContainer(c *dockertypes.Container) (*kubecontainer.Container, error) {
if c == nil {
return nil, fmt.Errorf("unable to convert a nil pointer to a runtime container")
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/kubelet/dockertools/convert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"reflect"
"testing"

dockertypes "github.com/docker/engine-api/types"
docker "github.com/fsouza/go-dockerclient"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
)
Expand All @@ -43,7 +44,7 @@ func TestMapState(t *testing.T) {
}

func TestToRuntimeContainer(t *testing.T) {
original := &docker.APIContainers{
original := &dockertypes.Container{
ID: "ab2cdf",
Image: "bar_image",
Created: 12345,
Expand Down
19 changes: 10 additions & 9 deletions pkg/kubelet/dockertools/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/docker/docker/pkg/jsonmessage"
dockerapi "github.com/docker/engine-api/client"
dockertypes "github.com/docker/engine-api/types"
docker "github.com/fsouza/go-dockerclient"
"github.com/golang/glog"
"k8s.io/kubernetes/pkg/api"
Expand Down Expand Up @@ -57,12 +58,12 @@ const (

// DockerInterface is an abstract interface for testability. It abstracts the interface of docker.Client.
type DockerInterface interface {
ListContainers(options docker.ListContainersOptions) ([]docker.APIContainers, error)
InspectContainer(id string) (*docker.Container, error)
CreateContainer(docker.CreateContainerOptions) (*docker.Container, error)
StartContainer(id string, hostConfig *docker.HostConfig) error
StopContainer(id string, timeout uint) error
RemoveContainer(opts docker.RemoveContainerOptions) error
ListContainers(options dockertypes.ContainerListOptions) ([]dockertypes.Container, error)
InspectContainer(id string) (*dockertypes.ContainerJSON, error)
CreateContainer(dockertypes.ContainerCreateConfig) (*dockertypes.ContainerCreateResponse, error)
StartContainer(id string) error
StopContainer(id string, timeout int) error
RemoveContainer(id string, opts dockertypes.ContainerRemoveOptions) error
InspectImage(image string) (*docker.Image, error)
ListImages(opts docker.ListImagesOptions) ([]docker.APIImages, error)
PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error
Expand Down Expand Up @@ -346,9 +347,9 @@ func milliCPUToShares(milliCPU int64) int64 {
// GetKubeletDockerContainers lists all container or just the running ones.
// Returns a list of docker containers that we manage
// TODO: Move this function with dockerCache to DockerManager.
func GetKubeletDockerContainers(client DockerInterface, allContainers bool) ([]*docker.APIContainers, error) {
result := []*docker.APIContainers{}
containers, err := client.ListContainers(docker.ListContainersOptions{All: allContainers})
func GetKubeletDockerContainers(client DockerInterface, allContainers bool) ([]*dockertypes.Container, error) {
result := []*dockertypes.Container{}
containers, err := client.ListContainers(dockertypes.ContainerListOptions{All: allContainers})
if err != nil {
return nil, err
}
Expand Down