diff --git a/pkg/api/v1/defaults.go b/pkg/api/v1/defaults.go index ac25b800831..938df8ec8f5 100644 --- a/pkg/api/v1/defaults.go +++ b/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 diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index 98e90db3e16..d265db51250 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/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". - imageID, tag, err := parsers.ParseImageName(image) + // If the image contains no tag or digest, a default tag should be applied. + 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{ - Tag: tag, - } + // The only used image pull option RegistryAuth will be set in kube_docker_client + 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 diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 4e9e388a920..2bf72610fbc 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/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) { - tests := []struct { - imageName string - name string - tag string +func TestApplyDefaultImageTag(t *testing.T) { + for _, testCase := range []struct { + Input string + Output string }{ - {"ubuntu", "ubuntu", "latest"}, - {"ubuntu:2342", "ubuntu", "2342"}, - {"ubuntu:latest", "ubuntu", "latest"}, - {"foo/bar:445566", "foo/bar", "445566"}, - {"registry.example.com:5000/foobar", "registry.example.com:5000/foobar", "latest"}, - {"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) + {Input: "root", Output: "root:latest"}, + {Input: "root:tag", Output: "root:tag"}, + {Input: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Output: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + } { + image, err := applyDefaultImageTag(testCase.Input) if err != nil { - t.Errorf("ParseImageName(%s) failed: %v", test.imageName, err) - } else if name != test.name || tag != test.tag { - t.Errorf("Expected name/tag: %s/%s, got %s/%s", test.name, test.tag, name, tag) + t.Errorf("applyDefaultTag(%s) failed: %v", testCase.Input, err) + } else if image != testCase.Output { + t.Errorf("Expected image reference: %q, got %q", testCase.Output, image) } } } diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index c0ea8ae0376..02f302fa03b 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/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 } diff --git a/pkg/kubelet/dockertools/kube_docker_client.go b/pkg/kubelet/dockertools/kube_docker_client.go index ad656d54a66..76bb89e548e 100644 --- a/pkg/kubelet/dockertools/kube_docker_client.go +++ b/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(), opts) + return d.client.ContainerRemove(getDefaultContext(), id, 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(), opts) + resp, err := d.client.ContainerLogs(getDefaultContext(), id, 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(), opts) + resp, err := d.client.ContainerExecCreate(getDefaultContext(), id, 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(), opts) + resp, err := d.client.ContainerAttach(getDefaultContext(), id, opts) if err != nil { return err } diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 9fc47030c95..20905461f2e 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/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 -} - -// compare is copied from engine-api, it compares two version strings, returns -1 if -// 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) + if dockerversion.LessThan(string(v), other) { + return -1, nil + } else if dockerversion.GreaterThan(string(v), other) { + return 1, nil } - for i := 0; i < max; i++ { - 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 + return 0, nil } func (dm *DockerManager) Type() string { diff --git a/pkg/kubelet/rkt/image.go b/pkg/kubelet/rkt/image.go index 47ea1a3627b..ae4d1554074 100644 --- a/pkg/kubelet/rkt/image.go +++ b/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 } diff --git a/pkg/util/parsers/parsers.go b/pkg/util/parsers/parsers.go index c173be66340..a02f18d3eb1 100644 --- a/pkg/util/parsers/parsers.go +++ b/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. -// If tag is empty, return the defaultImageTag. -func ParseImageName(image string) (string, string, error) { - named, err := reference.ParseNamed(image) +// ParseImageName parses a docker image string into three parts: repo, tag and digest. +// If both tag and digest are empty, a default image tag will be returned. +func ParseImageName(image string) (string, string, string, error) { + 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 { - tag = defaultImageTag + if len(tag) == 0 && len(digest) == 0 { + tag = DefaultImageTag } - return repoToPull, tag, nil + return repoToPull, tag, digest, nil } diff --git a/pkg/util/parsers/parsers_test.go b/pkg/util/parsers/parsers_test.go index 8c829682f20..371eee758a0 100644 --- a/pkg/util/parsers/parsers_test.go +++ b/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 - Repo string - Image string + Input string + Repo string + Tag string + Digest string }{ - {Input: "root", Repo: "root", Image: "latest"}, - {Input: "root:tag", Repo: "root", Image: "tag"}, - {Input: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "root", Image: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, - {Input: "user/repo", Repo: "user/repo", Image: "latest"}, - {Input: "user/repo:tag", Repo: "user/repo", Image: "tag"}, - {Input: "user/repo@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "user/repo", Image: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, - {Input: "url:5000/repo", Repo: "url:5000/repo", Image: "latest"}, - {Input: "url:5000/repo:tag", Repo: "url:5000/repo", Image: "tag"}, - {Input: "url:5000/repo@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "url:5000/repo", Image: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + {Input: "root", Repo: "root", Tag: "latest"}, + {Input: "root:tag", Repo: "root", Tag: "tag"}, + {Input: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "root", Digest: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + {Input: "user/repo", Repo: "user/repo", Tag: "latest"}, + {Input: "user/repo:tag", Repo: "user/repo", Tag: "tag"}, + {Input: "user/repo@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Repo: "user/repo", Digest: "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + {Input: "url:5000/repo", Repo: "url:5000/repo", Tag: "latest"}, + {Input: "url:5000/repo:tag", Repo: "url:5000/repo", Tag: "tag"}, + {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 { - t.Errorf("Expected repo: '%s' and image: '%s', got '%s' and '%s'", "root", "", repo, image) + } else if repo != testCase.Repo || tag != testCase.Tag || digest != testCase.Digest { + t.Errorf("Expected repo: %q, tag: %q and digest: %q, got %q, %q and %q", testCase.Repo, testCase.Tag, testCase.Digest, + repo, tag, digest) } } }