Refactor PullImage RemoveImage methods

Refactor image remove

pkg/credentialprovider/keyring.go

@@ -24,9 +24,9 @@ import (
 	"sort"
 	"strings"
 
-	docker "github.com/fsouza/go-dockerclient"
 	"github.com/golang/glog"
 
+	dockertypes "github.com/docker/engine-api/types"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/util/sets"
 )
@@ -54,17 +54,17 @@ type lazyDockerKeyring struct {
 	Providers []DockerConfigProvider
 }
 
-// LazyAuthConfiguration wraps AuthConfiguration, potentially deferring its
+// LazyAuthConfiguration wraps dockertypes.AuthConfig, potentially deferring its
 // binding. If Provider is non-nil, it will be used to obtain new credentials
 // by calling LazyProvide() on it.
 type LazyAuthConfiguration struct {
-	docker.AuthConfiguration
+	dockertypes.AuthConfig
 	Provider DockerConfigProvider
 }
 
 func DockerConfigEntryToLazyAuthConfiguration(ident DockerConfigEntry) LazyAuthConfiguration {
 	return LazyAuthConfiguration{
-		AuthConfiguration: docker.AuthConfiguration{
+		AuthConfig: dockertypes.AuthConfig{
 			Username: ident.Username,
 			Password: ident.Password,
 			Email:    ident.Email,
@@ -291,7 +291,6 @@ type unionDockerKeyring struct {
 
 func (k *unionDockerKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool) {
 	authConfigs := []LazyAuthConfiguration{}
-
 	for _, subKeyring := range k.keyrings {
 		if subKeyring == nil {
 			continue

pkg/credentialprovider/provider.go

@@ -22,7 +22,7 @@ import (
 	"sync"
 	"time"
 
-	docker "github.com/fsouza/go-dockerclient"
+	dockertypes "github.com/docker/engine-api/types"
 	"github.com/golang/glog"
 )
 
@@ -36,12 +36,12 @@ type DockerConfigProvider interface {
 	LazyProvide() *DockerConfigEntry
 }
 
-func LazyProvide(creds LazyAuthConfiguration) docker.AuthConfiguration {
+func LazyProvide(creds LazyAuthConfiguration) dockertypes.AuthConfig {
 	if creds.Provider != nil {
 		entry := *creds.Provider.LazyProvide()
-		return DockerConfigEntryToLazyAuthConfiguration(entry).AuthConfiguration
+		return DockerConfigEntryToLazyAuthConfiguration(entry).AuthConfig
 	} else {
-		return creds.AuthConfiguration
+		return creds.AuthConfig
 	}
 
 }

pkg/kubelet/dockertools/convert.go

@@ -21,7 +21,6 @@ import (
 	"strings"
 
 	dockertypes "github.com/docker/engine-api/types"
-	docker "github.com/fsouza/go-dockerclient"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 )
 

pkg/kubelet/dockertools/convert_test.go

@@ -21,7 +21,6 @@ import (
 	"testing"
 
 	dockertypes "github.com/docker/engine-api/types"
-	docker "github.com/fsouza/go-dockerclient"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 )
 

pkg/kubelet/dockertools/docker.go

@@ -27,7 +27,6 @@ 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"
 	"k8s.io/kubernetes/pkg/credentialprovider"
@@ -66,8 +65,8 @@ type DockerInterface interface {
 	RemoveContainer(id string, opts dockertypes.ContainerRemoveOptions) error
 	InspectImage(image string) (*dockertypes.ImageInspect, error)
 	ListImages(opts dockertypes.ImageListOptions) ([]dockertypes.Image, error)
-	PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error
+	PullImage(image string, auth dockertypes.AuthConfig, opts dockertypes.ImagePullOptions) error
-	RemoveImage(image string) error
+	RemoveImage(image string, opts dockertypes.ImageRemoveOptions) ([]dockertypes.ImageDelete, error)
 	Logs(string, dockertypes.ContainerLogsOptions, StreamOptions) error
 	Version() (*dockertypes.Version, error)
 	Info() (*dockertypes.Info, error)
@@ -146,23 +145,22 @@ func filterHTTPError(err error, image string) error {
 
 func (p dockerPuller) Pull(image string, secrets []api.Secret) error {
 	// If no tag was specified, use the default "latest".
-	repoToPull, tag := parsers.ParseImageName(image)
+	imageID, tag := parsers.ParseImageName(image)
-
-	opts := docker.PullImageOptions{
-		Repository: repoToPull,
-		Tag:        tag,
-	}
 
 	keyring, err := credentialprovider.MakeDockerKeyring(secrets, p.keyring)
 	if err != nil {
 		return err
 	}
 
-	creds, haveCredentials := keyring.Lookup(repoToPull)
+	opts := dockertypes.ImagePullOptions{
+		Tag: tag,
+	}
+
+	creds, haveCredentials := keyring.Lookup(imageID)
 	if !haveCredentials {
 		glog.V(1).Infof("Pulling image %s without credentials", image)
 
-		err := p.client.PullImage(opts, docker.AuthConfiguration{})
+		err := p.client.PullImage(imageID, dockertypes.AuthConfig{}, opts)
 		if err == nil {
 			// Sometimes PullImage failed with no error returned.
 			exist, ierr := p.IsImagePresent(image)
@@ -189,7 +187,7 @@ func (p dockerPuller) Pull(image string, secrets []api.Secret) error {
 
 	var pullErrs []error
 	for _, currentCreds := range creds {
-		err := p.client.PullImage(opts, credentialprovider.LazyProvide(currentCreds))
+		err = p.client.PullImage(imageID, credentialprovider.LazyProvide(currentCreds), opts)
 		// If there was no error, return success
 		if err == nil {
 			return nil
@@ -213,7 +211,7 @@ func (p dockerPuller) IsImagePresent(image string) (bool, error) {
 	if err == nil {
 		return true, nil
 	}
-	if err == docker.ErrNoSuchImage {
+	if _, ok := err.(imageNotFoundError); ok {
 		return false, nil
 	}
 	return false, err

pkg/kubelet/dockertools/docker_test.go

@@ -29,7 +29,6 @@ import (
 	"github.com/docker/docker/pkg/jsonmessage"
 	dockertypes "github.com/docker/engine-api/types"
 	dockernat "github.com/docker/go-connections/nat"
-	docker "github.com/fsouza/go-dockerclient"
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	"k8s.io/kubernetes/cmd/kubelet/app/options"
 	"k8s.io/kubernetes/pkg/api"
@@ -352,9 +351,8 @@ func TestDockerKeyringLookupFails(t *testing.T) {
 }
 
 func TestDockerKeyringLookup(t *testing.T) {
-
 	ada := credentialprovider.LazyAuthConfiguration{
-		AuthConfiguration: docker.AuthConfiguration{
+		AuthConfig: dockertypes.AuthConfig{
 			Username: "ada",
 			Password: "smash",
 			Email:    "ada@example.com",
@@ -362,7 +360,7 @@ func TestDockerKeyringLookup(t *testing.T) {
 	}
 
 	grace := credentialprovider.LazyAuthConfiguration{
-		AuthConfiguration: docker.AuthConfiguration{
+		AuthConfig: dockertypes.AuthConfig{
 			Username: "grace",
 			Password: "squash",
 			Email:    "grace@example.com",
@@ -425,7 +423,7 @@ func TestDockerKeyringLookup(t *testing.T) {
 // NOTE: the above covers the case of a more specific match trumping just hostname.
 func TestIssue3797(t *testing.T) {
 	rex := credentialprovider.LazyAuthConfiguration{
-		AuthConfiguration: docker.AuthConfiguration{
+		AuthConfig: dockertypes.AuthConfig{
 			Username: "rex",
 			Password: "tiny arms",
 			Email:    "rex@example.com",

pkg/kubelet/dockertools/fake_docker_client.go

@@ -28,7 +28,6 @@ import (
 
 	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"
 	"k8s.io/kubernetes/pkg/util/sets"
@@ -422,18 +421,14 @@ func (f *FakeDockerClient) Logs(id string, opts dockertypes.ContainerLogsOptions
 
 // 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 {
+func (f *FakeDockerClient) PullImage(imageID string, auth dockertypes.AuthConfig, opts dockertypes.ImagePullOptions) error {
 	f.Lock()
 	defer f.Unlock()
 	f.called = append(f.called, "pull")
 	err := f.popError("pull")
 	if err == nil {
-		registry := opts.Registry
-		if len(registry) != 0 {
-			registry = registry + "/"
-		}
 		authJson, _ := json.Marshal(auth)
-		f.pulled = append(f.pulled, fmt.Sprintf("%s%s:%s using %s", registry, opts.Repository, opts.Tag, string(authJson)))
+		f.pulled = append(f.pulled, fmt.Sprintf("%s:%s using %s", imageID, opts.Tag, string(authJson)))
 	}
 	return err
 }
@@ -479,12 +474,12 @@ func (f *FakeDockerClient) ListImages(opts dockertypes.ImageListOptions) ([]dock
 	return f.Images, err
 }
 
-func (f *FakeDockerClient) RemoveImage(image string) error {
+func (f *FakeDockerClient) RemoveImage(image string, opts dockertypes.ImageRemoveOptions) ([]dockertypes.ImageDelete, error) {
 	err := f.popError("remove_image")
 	if err == nil {
 		f.RemovedImages.Insert(image)
 	}
-	return err
+	return []dockertypes.ImageDelete{{Deleted: image}}, err
 }
 
 func (f *FakeDockerClient) updateContainerStatus(id, status string) {

pkg/kubelet/dockertools/instrumented_docker.go

@@ -20,7 +20,6 @@ import (
 	"time"
 
 	dockertypes "github.com/docker/engine-api/types"
-	docker "github.com/fsouza/go-dockerclient"
 	"k8s.io/kubernetes/pkg/kubelet/metrics"
 )
 
@@ -121,22 +120,21 @@ func (in instrumentedDockerInterface) ListImages(opts dockertypes.ImageListOptio
 	return out, err
 }
 
-func (in instrumentedDockerInterface) PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error {
+func (in instrumentedDockerInterface) PullImage(imageID string, auth dockertypes.AuthConfig, opts dockertypes.ImagePullOptions) error {
 	const operation = "pull_image"
 	defer recordOperation(operation, time.Now())
-
+	err := in.client.PullImage(imageID, auth, opts)
-	err := in.client.PullImage(opts, auth)
 	recordError(operation, err)
 	return err
 }
 
-func (in instrumentedDockerInterface) RemoveImage(image string) error {
+func (in instrumentedDockerInterface) RemoveImage(image string, opts dockertypes.ImageRemoveOptions) ([]dockertypes.ImageDelete, error) {
 	const operation = "remove_image"
 	defer recordOperation(operation, time.Now())
 
-	err := in.client.RemoveImage(image)
+	imageDelete, err := in.client.RemoveImage(image, opts)
 	recordError(operation, err)
-	return err
+	return imageDelete, err
 }
 
 func (in instrumentedDockerInterface) Logs(id string, opts dockertypes.ContainerLogsOptions, sopts StreamOptions) error {

pkg/kubelet/dockertools/kube_docker_client.go

@@ -30,7 +30,6 @@ import (
 	dockerapi "github.com/docker/engine-api/client"
 	dockertypes "github.com/docker/engine-api/types"
 	dockerfilters "github.com/docker/engine-api/types/filters"
-	docker "github.com/fsouza/go-dockerclient"
 	"golang.org/x/net/context"
 )
 
@@ -144,17 +143,12 @@ func (d *kubeDockerClient) RemoveContainer(id string, opts dockertypes.Container
 func (d *kubeDockerClient) InspectImage(image string) (*dockertypes.ImageInspect, error) {
 	resp, _, err := d.client.ImageInspectWithRaw(getDefaultContext(), image, true)
 	if err != nil {
-		// TODO(random-liu): Use IsErrImageNotFound instead of ErrNoSuchImage
 		if dockerapi.IsErrImageNotFound(err) {
-			err = docker.ErrNoSuchImage
+			err = imageNotFoundError{ID: image}
 		}
 		return nil, err
 	}
-	imageInfo := &dockertypes.ImageInspect{}
+	return &resp, nil
-	if err := convertType(&resp, imageInfo); err != nil {
-		return nil, err
-	}
-	return imageInfo, nil
 }
 
 func (d *kubeDockerClient) ListImages(opts dockertypes.ImageListOptions) ([]dockertypes.Image, error) {
@@ -162,14 +156,10 @@ func (d *kubeDockerClient) ListImages(opts dockertypes.ImageListOptions) ([]dock
 	if err != nil {
 		return nil, err
 	}
-	dockerImages := []dockertypes.Image{}
+	return images, nil
-	for _, img := range images {
-		dockerImages = append(dockerImages, dockertypes.Image(img))
-	}
-	return dockerImages, nil
 }
 
-func base64EncodeAuth(auth docker.AuthConfiguration) (string, error) {
+func base64EncodeAuth(auth dockertypes.AuthConfig) (string, error) {
 	var buf bytes.Buffer
 	if err := json.NewEncoder(&buf).Encode(auth); err != nil {
 		return "", err
@@ -177,16 +167,15 @@ func base64EncodeAuth(auth docker.AuthConfiguration) (string, error) {
 	return base64.URLEncoding.EncodeToString(buf.Bytes()), nil
 }
 
-func (d *kubeDockerClient) PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error {
+func (d *kubeDockerClient) PullImage(image string, auth dockertypes.AuthConfig, opts dockertypes.ImagePullOptions) error {
+	// RegistryAuth is the base64 encoded credentials for the registry
 	base64Auth, err := base64EncodeAuth(auth)
 	if err != nil {
 		return err
 	}
-	resp, err := d.client.ImagePull(getDefaultContext(), dockertypes.ImagePullOptions{
+	opts.ImageID = image
-		ImageID:      opts.Repository,
+	opts.RegistryAuth = base64Auth
-		Tag:          opts.Tag,
+	resp, err := d.client.ImagePull(getDefaultContext(), opts, nil)
-		RegistryAuth: base64Auth,
-	}, nil)
 	if err != nil {
 		return err
 	}
@@ -209,9 +198,8 @@ func (d *kubeDockerClient) PullImage(opts docker.PullImageOptions, auth docker.A
 	return nil
 }
 
-func (d *kubeDockerClient) RemoveImage(image string) error {
+func (d *kubeDockerClient) RemoveImage(image string, opts dockertypes.ImageRemoveOptions) ([]dockertypes.ImageDelete, error) {
-	_, err := d.client.ImageRemove(getDefaultContext(), dockertypes.ImageRemoveOptions{ImageID: image})
+	return d.client.ImageRemove(getDefaultContext(), dockertypes.ImageRemoveOptions{ImageID: image})
-	return err
 }
 
 func (d *kubeDockerClient) Logs(id string, opts dockertypes.ContainerLogsOptions, sopts StreamOptions) error {
@@ -355,3 +343,12 @@ type containerNotFoundError struct {
 func (e containerNotFoundError) Error() string {
 	return fmt.Sprintf("Error: No such container: %s", e.ID)
 }
+
+// imageNotFoundError is the error returned by InspectImage when image not found.
+type imageNotFoundError struct {
+	ID string
+}
+
+func (e imageNotFoundError) Error() string {
+	return fmt.Sprintf("Error: No such image: %s", e.ID)
+}

pkg/kubelet/dockertools/manager.go

@@ -34,7 +34,6 @@ import (
 	dockercontainer "github.com/docker/engine-api/types/container"
 	dockerstrslice "github.com/docker/engine-api/types/strslice"
 	dockernat "github.com/docker/go-connections/nat"
-	docker "github.com/fsouza/go-dockerclient"
 	"github.com/golang/glog"
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	"k8s.io/kubernetes/pkg/api"
@@ -821,7 +820,9 @@ func (dm *DockerManager) IsImagePresent(image kubecontainer.ImageSpec) (bool, er
 
 // Removes the specified image.
 func (dm *DockerManager) RemoveImage(image kubecontainer.ImageSpec) error {
-	return dm.client.RemoveImage(image.Image)
+	// TODO(harryz) currently Runtime interface does not provide other remove options.
+	_, err := dm.client.RemoveImage(image.Image, dockertypes.ImageRemoveOptions{})
+	return err
 }
 
 // podInfraContainerChanged returns true if the pod infra container has changed.

pkg/kubelet/dockertools/manager_test.go

@@ -30,10 +30,8 @@ import (
 	"time"
 
 	dockertypes "github.com/docker/engine-api/types"
-	containertypes "github.com/docker/engine-api/types/container"
 	dockercontainer "github.com/docker/engine-api/types/container"
 	dockerstrslice "github.com/docker/engine-api/types/strslice"
-	docker "github.com/fsouza/go-dockerclient"
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/kubernetes/cmd/kubelet/app/options"
@@ -1430,7 +1428,7 @@ func TestVerifyNonRoot(t *testing.T) {
 		"numeric non-root image user": {
 			container: &api.Container{},
 			inspectImage: &dockertypes.ImageInspect{
-				Config: &containertypes.Config{
+				Config: &dockercontainer.Config{
 					User: "1",
 				},
 			},
@@ -1438,7 +1436,7 @@ func TestVerifyNonRoot(t *testing.T) {
 		"numeric non-root image user with gid": {
 			container: &api.Container{},
 			inspectImage: &dockertypes.ImageInspect{
-				Config: &containertypes.Config{
+				Config: &dockercontainer.Config{
 					User: "1:2",
 				},
 			},
@@ -1456,7 +1454,7 @@ func TestVerifyNonRoot(t *testing.T) {
 		"non-numeric image user": {
 			container: &api.Container{},
 			inspectImage: &dockertypes.ImageInspect{
-				Config: &containertypes.Config{
+				Config: &dockercontainer.Config{
 					User: "foo",
 				},
 			},
@@ -1465,7 +1463,7 @@ func TestVerifyNonRoot(t *testing.T) {
 		"numeric root image user": {
 			container: &api.Container{},
 			inspectImage: &dockertypes.ImageInspect{
-				Config: &containertypes.Config{
+				Config: &dockercontainer.Config{
 					User: "0",
 				},
 			},
@@ -1474,7 +1472,7 @@ func TestVerifyNonRoot(t *testing.T) {
 		"numeric root image user with gid": {
 			container: &api.Container{},
 			inspectImage: &dockertypes.ImageInspect{
-				Config: &containertypes.Config{
+				Config: &dockercontainer.Config{
 					User: "0:1",
 				},
 			},

pkg/kubelet/rkt/image.go

@@ -28,7 +28,7 @@ import (
 
 	appcschema "github.com/appc/spec/schema"
 	rktapi "github.com/coreos/rkt/api/v1alpha"
-	"github.com/fsouza/go-dockerclient"
+	dockertypes "github.com/docker/engine-api/types"
 	"github.com/golang/glog"
 	"golang.org/x/net/context"
 	"k8s.io/kubernetes/pkg/api"
@@ -183,7 +183,7 @@ func (r *Runtime) writeDockerAuthConfig(image string, credsSlice []credentialpro
 		return nil
 	}
 
-	creds := docker.AuthConfiguration{}
+	creds := dockertypes.AuthConfig{}
 	// TODO handle multiple creds
 	if len(credsSlice) >= 1 {
 		creds = credentialprovider.LazyProvide(credsSlice[0])