From d3d98b372b461002cccdcc72ab0fd674a5f4aa0f Mon Sep 17 00:00:00 2001 From: Random-Liu Date: Thu, 14 Apr 2016 10:37:35 -0700 Subject: [PATCH] Refactor StartContainer, StopContainer and RemoveContainer. --- contrib/mesos/pkg/executor/executor.go | 9 ++++----- pkg/kubelet/dockertools/container_gc.go | 6 +++--- pkg/kubelet/dockertools/docker.go | 6 +++--- pkg/kubelet/dockertools/fake_docker_client.go | 18 +++++++----------- pkg/kubelet/dockertools/instrumented_docker.go | 10 +++++----- pkg/kubelet/dockertools/kube_docker_client.go | 16 ++++++---------- pkg/kubelet/dockertools/manager.go | 4 ++-- 7 files changed, 30 insertions(+), 39 deletions(-) diff --git a/contrib/mesos/pkg/executor/executor.go b/contrib/mesos/pkg/executor/executor.go index c32083c617c..3dfe68d14d2 100644 --- a/contrib/mesos/pkg/executor/executor.go +++ b/contrib/mesos/pkg/executor/executor.go @@ -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" @@ -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) } } diff --git a/pkg/kubelet/dockertools/container_gc.go b/pkg/kubelet/dockertools/container_gc.go index 9d957dacdb3..22319f7fdbe 100644 --- a/pkg/kubelet/dockertools/container_gc.go +++ b/pkg/kubelet/dockertools/container_gc.go @@ -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" @@ -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) } @@ -195,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) } diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index cbd0a48dd04..a5c5adfbce3 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -61,9 +61,9 @@ type DockerInterface interface { ListContainers(options dockertypes.ContainerListOptions) ([]dockertypes.Container, error) InspectContainer(id string) (*dockertypes.ContainerJSON, error) CreateContainer(dockertypes.ContainerCreateConfig) (*dockertypes.ContainerCreateResponse, error) - StartContainer(id string, hostConfig *docker.HostConfig) error - StopContainer(id string, timeout uint) error - RemoveContainer(opts docker.RemoveContainerOptions) 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 diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index 546329c6c6b..eb52bfcd162 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/pkg/kubelet/dockertools/fake_docker_client.go @@ -328,11 +328,7 @@ func (f *FakeDockerClient) CreateContainer(c dockertypes.ContainerCreateConfig) // StartContainer is a test-spy implementation of DockerInterface.StartContainer. // It adds an entry "start" to the internal method call record. -// The HostConfig at StartContainer will be deprecated from docker 1.10. Now in -// docker manager the HostConfig is set when ContainerCreate(). -// TODO(random-liu): Remove the HostConfig here when it is completely removed in -// docker 1.12. -func (f *FakeDockerClient) StartContainer(id string, _ *docker.HostConfig) error { +func (f *FakeDockerClient) StartContainer(id string) error { f.Lock() defer f.Unlock() f.called = append(f.called, "start") @@ -355,7 +351,7 @@ func (f *FakeDockerClient) StartContainer(id string, _ *docker.HostConfig) error // StopContainer is a test-spy implementation of DockerInterface.StopContainer. // It adds an entry "stop" to the internal method call record. -func (f *FakeDockerClient) StopContainer(id string, timeout uint) error { +func (f *FakeDockerClient) StopContainer(id string, timeout int) error { f.Lock() defer f.Unlock() f.called = append(f.called, "stop") @@ -393,7 +389,7 @@ func (f *FakeDockerClient) StopContainer(id string, timeout uint) error { return nil } -func (f *FakeDockerClient) RemoveContainer(opts docker.RemoveContainerOptions) error { +func (f *FakeDockerClient) RemoveContainer(id string, opts dockertypes.ContainerRemoveOptions) error { f.Lock() defer f.Unlock() f.called = append(f.called, "remove") @@ -402,10 +398,10 @@ func (f *FakeDockerClient) RemoveContainer(opts docker.RemoveContainerOptions) e return err } for i := range f.ExitedContainerList { - if f.ExitedContainerList[i].ID == opts.ID { - delete(f.ContainerMap, opts.ID) + if f.ExitedContainerList[i].ID == id { + delete(f.ContainerMap, id) f.ExitedContainerList = append(f.ExitedContainerList[:i], f.ExitedContainerList[i+1:]...) - f.Removed = append(f.Removed, opts.ID) + f.Removed = append(f.Removed, id) return nil } @@ -423,7 +419,7 @@ func (f *FakeDockerClient) Logs(opts docker.LogsOptions) error { return f.popError("logs") } -// PullImage is a test-spy implementation of DockerInterface.StopContainer. +// PullImage is a test-spy implementation of DockerInterface.PullImage. // It adds an entry "pull" to the internal method call record. func (f *FakeDockerClient) PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error { f.Lock() diff --git a/pkg/kubelet/dockertools/instrumented_docker.go b/pkg/kubelet/dockertools/instrumented_docker.go index 7c9682e42b5..c9053132c3f 100644 --- a/pkg/kubelet/dockertools/instrumented_docker.go +++ b/pkg/kubelet/dockertools/instrumented_docker.go @@ -76,16 +76,16 @@ func (in instrumentedDockerInterface) CreateContainer(opts dockertypes.Container return out, err } -func (in instrumentedDockerInterface) StartContainer(id string, hostConfig *docker.HostConfig) error { +func (in instrumentedDockerInterface) StartContainer(id string) error { const operation = "start_container" defer recordOperation(operation, time.Now()) - err := in.client.StartContainer(id, hostConfig) + err := in.client.StartContainer(id) recordError(operation, err) return err } -func (in instrumentedDockerInterface) StopContainer(id string, timeout uint) error { +func (in instrumentedDockerInterface) StopContainer(id string, timeout int) error { const operation = "stop_container" defer recordOperation(operation, time.Now()) @@ -94,11 +94,11 @@ func (in instrumentedDockerInterface) StopContainer(id string, timeout uint) err return err } -func (in instrumentedDockerInterface) RemoveContainer(opts docker.RemoveContainerOptions) error { +func (in instrumentedDockerInterface) RemoveContainer(id string, opts dockertypes.ContainerRemoveOptions) error { const operation = "remove_container" defer recordOperation(operation, time.Now()) - err := in.client.RemoveContainer(opts) + err := in.client.RemoveContainer(id, opts) recordError(operation, err) return err } diff --git a/pkg/kubelet/dockertools/kube_docker_client.go b/pkg/kubelet/dockertools/kube_docker_client.go index 959d3c65b66..60f8dd08055 100644 --- a/pkg/kubelet/dockertools/kube_docker_client.go +++ b/pkg/kubelet/dockertools/kube_docker_client.go @@ -132,22 +132,18 @@ func (d *kubeDockerClient) CreateContainer(opts dockertypes.ContainerCreateConfi return &createResp, nil } -// TODO(random-liu): The HostConfig at container start is deprecated, will remove this in the following refactoring. -func (d *kubeDockerClient) StartContainer(id string, _ *docker.HostConfig) error { +func (d *kubeDockerClient) StartContainer(id string) error { return d.client.ContainerStart(getDefaultContext(), id) } // Stopping an already stopped container will not cause an error in engine-api. -func (d *kubeDockerClient) StopContainer(id string, timeout uint) error { - return d.client.ContainerStop(getDefaultContext(), id, int(timeout)) +func (d *kubeDockerClient) StopContainer(id string, timeout int) error { + return d.client.ContainerStop(getDefaultContext(), id, timeout) } -func (d *kubeDockerClient) RemoveContainer(opts docker.RemoveContainerOptions) error { - return d.client.ContainerRemove(getDefaultContext(), dockertypes.ContainerRemoveOptions{ - ContainerID: opts.ID, - RemoveVolumes: opts.RemoveVolumes, - Force: opts.Force, - }) +func (d *kubeDockerClient) RemoveContainer(id string, opts dockertypes.ContainerRemoveOptions) error { + opts.ContainerID = id + return d.client.ContainerRemove(getDefaultContext(), opts) } func (d *kubeDockerClient) InspectImage(image string) (*docker.Image, error) { diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index e685f3a9054..7deacedf075 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -674,7 +674,7 @@ func (dm *DockerManager) runContainer( } dm.recorder.Eventf(ref, api.EventTypeNormal, kubecontainer.CreatedContainer, "Created container with docker id %v", utilstrings.ShortenString(createResp.ID, 12)) - if err = dm.client.StartContainer(createResp.ID, nil); err != nil { + if err = dm.client.StartContainer(createResp.ID); err != nil { dm.recorder.Eventf(ref, api.EventTypeWarning, kubecontainer.FailedToStartContainer, "Failed to start container with docker id %v with error: %v", utilstrings.ShortenString(createResp.ID, 12), err) return kubecontainer.ContainerID{}, err @@ -1397,7 +1397,7 @@ func (dm *DockerManager) killContainer(containerID kubecontainer.ContainerID, co if gracePeriod < minimumGracePeriodInSeconds { gracePeriod = minimumGracePeriodInSeconds } - err := dm.client.StopContainer(ID, uint(gracePeriod)) + err := dm.client.StopContainer(ID, int(gracePeriod)) if err == nil { glog.V(2).Infof("Container %q exited after %s", name, unversioned.Now().Sub(start.Time)) } else {