Cleanup the code with new engine-api

pkg/api/v1/defaults.go

@@ -92,7 +92,7 @@ func SetDefaults_ContainerPort(obj *ContainerPort) {
 func SetDefaults_Container(obj *Container) {
 	if obj.ImagePullPolicy == "" {
 		// Ignore error and assume it has been validated elsewhere
-		_, tag, _ := parsers.ParseImageName(obj.Image)
+		_, tag, _, _ := parsers.ParseImageName(obj.Image)
 
 		// Check image tag
 

pkg/kubelet/dockertools/docker.go

@@ -24,6 +24,7 @@ import (
 	"strconv"
 	"strings"
 
+	dockerref "github.com/docker/distribution/reference"
 	"github.com/docker/docker/pkg/jsonmessage"
 	dockerapi "github.com/docker/engine-api/client"
 	dockertypes "github.com/docker/engine-api/types"
@@ -144,9 +145,28 @@ func filterHTTPError(err error, image string) error {
 	}
 }
 
+// applyDefaultImageTag parses a docker image string, if it doesn't contain any tag or digest,
+// a default tag will be applied.
+func applyDefaultImageTag(image string) (string, error) {
+	named, err := dockerref.ParseNamed(image)
+	if err != nil {
+		return "", fmt.Errorf("couldn't parse image reference %q: %v", image, err)
+	}
+	_, isTagged := named.(dockerref.Tagged)
+	_, isDigested := named.(dockerref.Digested)
+	if !isTagged && !isDigested {
+		named, err := dockerref.WithTag(named, parsers.DefaultImageTag)
+		if err != nil {
+			return "", fmt.Errorf("failed to apply default image tag %q: %v", image, err)
+		}
+		image = named.String()
+	}
+	return image, nil
+}
+
 func (p dockerPuller) Pull(image string, secrets []api.Secret) error {
-	// If no tag was specified, use the default "latest".
+	// If the image contains no tag or digest, a default tag should be applied.
-	imageID, tag, err := parsers.ParseImageName(image)
+	image, err := applyDefaultImageTag(image)
 	if err != nil {
 		return err
 	}
@@ -156,15 +176,14 @@ func (p dockerPuller) Pull(image string, secrets []api.Secret) error {
 		return err
 	}
 
-	opts := dockertypes.ImagePullOptions{
+	// The only used image pull option RegistryAuth will be set in kube_docker_client
-		Tag: tag,
+	opts := dockertypes.ImagePullOptions{}
-	}
 
-	creds, haveCredentials := keyring.Lookup(imageID)
+	creds, haveCredentials := keyring.Lookup(image)
 	if !haveCredentials {
 		glog.V(1).Infof("Pulling image %s without credentials", image)
 
-		err := p.client.PullImage(imageID, dockertypes.AuthConfig{}, opts)
+		err := p.client.PullImage(image, dockertypes.AuthConfig{}, opts)
 		if err == nil {
 			// Sometimes PullImage failed with no error returned.
 			exist, ierr := p.IsImagePresent(image)
@@ -191,7 +210,7 @@ func (p dockerPuller) Pull(image string, secrets []api.Secret) error {
 
 	var pullErrs []error
 	for _, currentCreds := range creds {
-		err = p.client.PullImage(imageID, credentialprovider.LazyProvide(currentCreds), opts)
+		err = p.client.PullImage(image, credentialprovider.LazyProvide(currentCreds), opts)
 		// If there was no error, return success
 		if err == nil {
 			return nil

pkg/kubelet/dockertools/docker_test.go

@@ -40,7 +40,6 @@ import (
 	nettest "k8s.io/kubernetes/pkg/kubelet/network/testing"
 	"k8s.io/kubernetes/pkg/types"
 	hashutil "k8s.io/kubernetes/pkg/util/hash"
-	"k8s.io/kubernetes/pkg/util/parsers"
 )
 
 func verifyCalls(t *testing.T, fakeDocker *FakeDockerClient, calls []string) {
@@ -156,26 +155,20 @@ func TestContainerNaming(t *testing.T) {
 	}
 }
 
-func TestParseImageName(t *testing.T) {
+func TestApplyDefaultImageTag(t *testing.T) {
-	tests := []struct {
+	for _, testCase := range []struct {
-		imageName string
+		Input  string
-		name      string
+		Output string
-		tag       string
 	}{
-		{"ubuntu", "ubuntu", "latest"},
+		{Input: "root", Output: "root:latest"},
-		{"ubuntu:2342", "ubuntu", "2342"},
+		{Input: "root:tag", Output: "root:tag"},
-		{"ubuntu:latest", "ubuntu", "latest"},
+		{Input: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Output: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
-		{"foo/bar:445566", "foo/bar", "445566"},
+	} {
-		{"registry.example.com:5000/foobar", "registry.example.com:5000/foobar", "latest"},
+		image, err := applyDefaultImageTag(testCase.Input)
-		{"registry.example.com:5000/foobar:5342", "registry.example.com:5000/foobar", "5342"},
-		{"registry.example.com:5000/foobar:latest", "registry.example.com:5000/foobar", "latest"},
-	}
-	for _, test := range tests {
-		name, tag, err := parsers.ParseImageName(test.imageName)
 		if err != nil {
-			t.Errorf("ParseImageName(%s) failed: %v", test.imageName, err)
+			t.Errorf("applyDefaultTag(%s) failed: %v", testCase.Input, err)
-		} else if name != test.name || tag != test.tag {
+		} else if image != testCase.Output {
-			t.Errorf("Expected name/tag: %s/%s, got %s/%s", test.name, test.tag, name, tag)
+			t.Errorf("Expected image reference: %q, got %q", testCase.Output, image)
 		}
 	}
 }

pkg/kubelet/dockertools/fake_docker_client.go

@@ -422,14 +422,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(imageID string, auth dockertypes.AuthConfig, opts dockertypes.ImagePullOptions) error {
+func (f *FakeDockerClient) PullImage(image 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 {
 		authJson, _ := json.Marshal(auth)
-		f.pulled = append(f.pulled, fmt.Sprintf("%s:%s using %s", imageID, opts.Tag, string(authJson)))
+		f.pulled = append(f.pulled, fmt.Sprintf("%s using %s", image, string(authJson)))
 	}
 	return err
 }

pkg/kubelet/dockertools/kube_docker_client.go

@@ -29,7 +29,6 @@ import (
 	dockerstdcopy "github.com/docker/docker/pkg/stdcopy"
 	dockerapi "github.com/docker/engine-api/client"
 	dockertypes "github.com/docker/engine-api/types"
-	dockerfilters "github.com/docker/engine-api/types/filters"
 	"golang.org/x/net/context"
 )
 
@@ -70,26 +69,6 @@ func getDefaultContext() context.Context {
 	return context.Background()
 }
 
-// convertType converts between different types with the same json format.
-func convertType(src interface{}, dst interface{}) error {
-	data, err := json.Marshal(src)
-	if err != nil {
-		return err
-	}
-	return json.Unmarshal(data, dst)
-}
-
-// convertFilters converts filters to the filter type in engine-api.
-func convertFilters(filters map[string][]string) dockerfilters.Args {
-	args := dockerfilters.NewArgs()
-	for name, fields := range filters {
-		for _, field := range fields {
-			args.Add(name, field)
-		}
-	}
-	return args
-}
-
 func (k *kubeDockerClient) ListContainers(options dockertypes.ContainerListOptions) ([]dockertypes.Container, error) {
 	containers, err := k.client.ContainerList(getDefaultContext(), options)
 	if err != nil {
@@ -136,8 +115,7 @@ func (d *kubeDockerClient) StopContainer(id string, timeout int) error {
 }
 
 func (d *kubeDockerClient) RemoveContainer(id string, opts dockertypes.ContainerRemoveOptions) error {
-	opts.ContainerID = id
+	return d.client.ContainerRemove(getDefaultContext(), id, opts)
-	return d.client.ContainerRemove(getDefaultContext(), opts)
 }
 
 func (d *kubeDockerClient) InspectImage(image string) (*dockertypes.ImageInspect, error) {
@@ -177,9 +155,8 @@ func (d *kubeDockerClient) PullImage(image string, auth dockertypes.AuthConfig,
 	if err != nil {
 		return err
 	}
-	opts.ImageID = image
 	opts.RegistryAuth = base64Auth
-	resp, err := d.client.ImagePull(getDefaultContext(), opts, nil)
+	resp, err := d.client.ImagePull(getDefaultContext(), image, opts)
 	if err != nil {
 		return err
 	}
@@ -203,12 +180,11 @@ func (d *kubeDockerClient) PullImage(image string, auth dockertypes.AuthConfig,
 }
 
 func (d *kubeDockerClient) RemoveImage(image string, opts dockertypes.ImageRemoveOptions) ([]dockertypes.ImageDelete, error) {
-	return d.client.ImageRemove(getDefaultContext(), dockertypes.ImageRemoveOptions{ImageID: image})
+	return d.client.ImageRemove(getDefaultContext(), image, opts)
 }
 
 func (d *kubeDockerClient) Logs(id string, opts dockertypes.ContainerLogsOptions, sopts StreamOptions) error {
-	opts.ContainerID = id
+	resp, err := d.client.ContainerLogs(getDefaultContext(), id, opts)
-	resp, err := d.client.ContainerLogs(getDefaultContext(), opts)
 	if err != nil {
 		return err
 	}
@@ -234,8 +210,7 @@ func (d *kubeDockerClient) Info() (*dockertypes.Info, error) {
 
 // TODO(random-liu): Add unit test for exec and attach functions, just like what go-dockerclient did.
 func (d *kubeDockerClient) CreateExec(id string, opts dockertypes.ExecConfig) (*dockertypes.ContainerExecCreateResponse, error) {
-	opts.Container = id
+	resp, err := d.client.ContainerExecCreate(getDefaultContext(), id, opts)
-	resp, err := d.client.ContainerExecCreate(getDefaultContext(), opts)
 	if err != nil {
 		return nil, err
 	}
@@ -266,8 +241,7 @@ func (d *kubeDockerClient) InspectExec(id string) (*dockertypes.ContainerExecIns
 }
 
 func (d *kubeDockerClient) AttachToContainer(id string, opts dockertypes.ContainerAttachOptions, sopts StreamOptions) error {
-	opts.ContainerID = id
+	resp, err := d.client.ContainerAttach(getDefaultContext(), id, opts)
-	resp, err := d.client.ContainerAttach(getDefaultContext(), opts)
 	if err != nil {
 		return err
 	}

pkg/kubelet/dockertools/manager.go

@@ -33,6 +33,7 @@ import (
 	dockertypes "github.com/docker/engine-api/types"
 	dockercontainer "github.com/docker/engine-api/types/container"
 	dockerstrslice "github.com/docker/engine-api/types/strslice"
+	dockerversion "github.com/docker/engine-api/types/versions"
 	dockernat "github.com/docker/go-connections/nat"
 	"github.com/golang/glog"
 	cadvisorapi "github.com/google/cadvisor/info/v1"
@@ -894,40 +895,12 @@ func (v dockerVersion) String() string {
 }
 
 func (v dockerVersion) Compare(other string) (int, error) {
-	return compare(string(v), other), nil
+	if dockerversion.LessThan(string(v), other) {
-}
+		return -1, nil
-
+	} else if dockerversion.GreaterThan(string(v), other) {
-// compare is copied from engine-api, it compares two version strings, returns -1 if
+		return 1, nil
-// v1 < v2, 1 if v1 > v2, 0 otherwise.
-// TODO(random-liu): Leveraging the version comparison in engine-api after bumping up
-// the engine-api version. See #24076
-func compare(v1, v2 string) int {
-	var (
-		currTab  = strings.Split(v1, ".")
-		otherTab = strings.Split(v2, ".")
-	)
-
-	max := len(currTab)
-	if len(otherTab) > max {
-		max = len(otherTab)
 	}
-	for i := 0; i < max; i++ {
+	return 0, nil
-		var currInt, otherInt int
-
-		if len(currTab) > i {
-			currInt, _ = strconv.Atoi(currTab[i])
-		}
-		if len(otherTab) > i {
-			otherInt, _ = strconv.Atoi(otherTab[i])
-		}
-		if currInt > otherInt {
-			return 1
-		}
-		if otherInt > currInt {
-			return -1
-		}
-	}
-	return 0
 }
 
 func (dm *DockerManager) Type() string {

pkg/kubelet/rkt/image.go

@@ -47,7 +47,7 @@ func (r *Runtime) PullImage(image kubecontainer.ImageSpec, pullSecrets []api.Sec
 	img := image.Image
 	// TODO(yifan): The credential operation is a copy from dockertools package,
 	// Need to resolve the code duplication.
-	repoToPull, _, err := parsers.ParseImageName(img)
+	repoToPull, _, _, err := parsers.ParseImageName(img)
 	if err != nil {
 		return err
 	}
@@ -142,7 +142,7 @@ func (s sortByImportTime) Less(i, j int) bool { return s[i].ImportTimestamp < s[
 // will return the result reversely sorted by the import time, so that the latest
 // image comes first.
 func (r *Runtime) listImages(image string, detail bool) ([]*rktapi.Image, error) {
-	repoToPull, tag, err := parsers.ParseImageName(image)
+	repoToPull, tag, _, err := parsers.ParseImageName(image)
 	if err != nil {
 		return nil, err
 	}

pkg/util/parsers/parsers.go

@@ -19,37 +19,36 @@ package parsers
 import (
 	"fmt"
 
-	"github.com/docker/distribution/reference"
+	dockerref "github.com/docker/distribution/reference"
 )
 
 const (
-	defaultImageTag = "latest"
+	DefaultImageTag = "latest"
 )
 
-// parseImageName parses a docker image string into two parts: repo and tag.
+// ParseImageName parses a docker image string into three parts: repo, tag and digest.
-// If tag is empty, return the defaultImageTag.
+// If both tag and digest are empty, a default image tag will be returned.
-func ParseImageName(image string) (string, string, error) {
+func ParseImageName(image string) (string, string, string, error) {
-	named, err := reference.ParseNamed(image)
+	named, err := dockerref.ParseNamed(image)
 	if err != nil {
-		return "", "", fmt.Errorf("couldn't parse image name: %v", err)
+		return "", "", "", fmt.Errorf("couldn't parse image name: %v", err)
 	}
 
 	repoToPull := named.Name()
-	var tag string
+	var tag, digest string
 
-	tagged, ok := named.(reference.Tagged)
+	tagged, ok := named.(dockerref.Tagged)
 	if ok {
 		tag = tagged.Tag()
 	}
 
-	digested, ok := named.(reference.Digested)
+	digested, ok := named.(dockerref.Digested)
 	if ok {
-		tag = digested.Digest().String()
+		digest = digested.Digest().String()
 	}
-
 	// If no tag was specified, use the default "latest".
-	if len(tag) == 0 {
+	if len(tag) == 0 && len(digest) == 0 {
-		tag = defaultImageTag
+		tag = DefaultImageTag
 	}
-	return repoToPull, tag, nil
+	return repoToPull, tag, digest, nil
 }

pkg/util/parsers/parsers_test.go

@@ -24,26 +24,28 @@ import (
 // https://github.com/docker/docker/commit/4352da7803d182a6013a5238ce20a7c749db979a
 func TestParseImageName(t *testing.T) {
 	testCases := []struct {
-		Input string
+		Input  string
-		Repo  string
+		Repo   string
-		Image string
+		Tag    string
+		Digest string
 	}{
-		{Input: "root", Repo: "root", Image: "latest"},
+		{Input: "root", Repo: "root", Tag: "latest"},
-		{Input: "root:tag", Repo: "root", Image: "tag"},
+		{Input: "root:tag", Repo: "root", Tag: "tag"},
-		{Input: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "root", Image: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
+		{Input: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "root", Digest: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
-		{Input: "user/repo", Repo: "user/repo", Image: "latest"},
+		{Input: "user/repo", Repo: "user/repo", Tag: "latest"},
-		{Input: "user/repo:tag", Repo: "user/repo", Image: "tag"},
+		{Input: "user/repo:tag", Repo: "user/repo", Tag: "tag"},
-		{Input: "user/repo@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "user/repo", Image: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
+		{Input: "user/repo@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "user/repo", Digest: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
-		{Input: "url:5000/repo", Repo: "url:5000/repo", Image: "latest"},
+		{Input: "url:5000/repo", Repo: "url:5000/repo", Tag: "latest"},
-		{Input: "url:5000/repo:tag", Repo: "url:5000/repo", Image: "tag"},
+		{Input: "url:5000/repo:tag", Repo: "url:5000/repo", Tag: "tag"},
-		{Input: "url:5000/repo@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "url:5000/repo", Image: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
+		{Input: "url:5000/repo@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "url:5000/repo", Digest: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
 	}
 	for _, testCase := range testCases {
-		repo, image, err := ParseImageName(testCase.Input)
+		repo, tag, digest, err := ParseImageName(testCase.Input)
 		if err != nil {
 			t.Errorf("ParseImageName(%s) failed: %v", testCase.Input, err)
-		} else if repo != testCase.Repo || image != testCase.Image {
+		} else if repo != testCase.Repo || tag != testCase.Tag || digest != testCase.Digest {
-			t.Errorf("Expected repo: '%s' and image: '%s', got '%s' and '%s'", "root", "", repo, image)
+			t.Errorf("Expected repo: %q, tag: %q and digest: %q, got %q, %q and %q", testCase.Repo, testCase.Tag, testCase.Digest,
+				repo, tag, digest)
 		}
 	}
 }