From 121c7046aa58ff80cbcfbd8c0783dbe3033490b2 Mon Sep 17 00:00:00 2001 From: harry zhang Date: Sun, 15 Nov 2015 15:37:59 +0800 Subject: [PATCH 1/2] Move parsers into util --- pkg/kubelet/dockertools/docker.go | 8 ++---- pkg/kubelet/dockertools/docker_test.go | 7 ++--- pkg/kubelet/rkt/rkt.go | 19 +++----------- pkg/util/parsers/parsers.go | 36 ++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 24 deletions(-) create mode 100644 pkg/util/parsers/parsers.go diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index 6fe8ce5bf03..640742e1842 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -25,7 +25,6 @@ import ( "strings" "github.com/docker/docker/pkg/jsonmessage" - "github.com/docker/docker/pkg/parsers" docker "github.com/fsouza/go-dockerclient" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" @@ -35,6 +34,7 @@ import ( "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util" utilerrors "k8s.io/kubernetes/pkg/util/errors" + "k8s.io/kubernetes/pkg/util/parsers" ) const ( @@ -115,10 +115,6 @@ func newDockerPuller(client DockerInterface, qps float32, burst int) DockerPulle } } -func parseImageName(image string) (string, string) { - return parsers.ParseRepositoryTag(image) -} - func filterHTTPError(err error, image string) error { // docker/docker/pull/11314 prints detailed error info for docker pull. // When it hits 502, it returns a verbose html output including an inline svg, @@ -136,7 +132,7 @@ func filterHTTPError(err error, image string) error { } func (p dockerPuller) Pull(image string, secrets []api.Secret) error { - repoToPull, tag := parseImageName(image) + repoToPull, tag := parsers.ParseImageName(image) // If no tag was specified, use the default "latest". if len(tag) == 0 { diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index d0a5e20e893..af911a45862 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -37,6 +37,7 @@ import ( kubetypes "k8s.io/kubernetes/pkg/kubelet/types" "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util" + "k8s.io/kubernetes/pkg/util/parsers" ) func verifyCalls(t *testing.T, fakeDocker *FakeDockerClient, calls []string) { @@ -204,16 +205,16 @@ func TestParseImageName(t *testing.T) { name string tag string }{ - {"ubuntu", "ubuntu", ""}, + {"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", ""}, + {"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 := parseImageName(test.imageName) + name, tag := parsers.ParseImageName(test.imageName) if name != test.name || tag != test.tag { t.Errorf("Expected name/tag: %s/%s, got %s/%s", test.name, test.tag, name, tag) } diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index 5bfe3fa8e8b..78eb5a24f40 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -36,7 +36,6 @@ import ( appctypes "github.com/appc/spec/schema/types" "github.com/coreos/go-systemd/unit" rktapi "github.com/coreos/rkt/api/v1alpha" - "github.com/docker/docker/pkg/parsers" docker "github.com/fsouza/go-dockerclient" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" @@ -49,6 +48,7 @@ import ( "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util" utilexec "k8s.io/kubernetes/pkg/util/exec" + "k8s.io/kubernetes/pkg/util/parsers" "k8s.io/kubernetes/pkg/util/sets" ) @@ -389,23 +389,12 @@ func setApp(app *appctypes.App, c *api.Container, opts *kubecontainer.RunContain return setIsolators(app, c) } -// 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) { - repoToPull, tag := parsers.ParseRepositoryTag(image) - // If no tag was specified, use the default "latest". - if len(tag) == 0 { - tag = defaultImageTag - } - return repoToPull, tag -} - // getImageManifest invokes 'rkt image cat-manifest' to retrive the image manifest // for the image. func (r *Runtime) getImageManifest(image string) (*appcschema.ImageManifest, error) { var manifest appcschema.ImageManifest - repoToPull, tag := parseImageName(image) + repoToPull, tag := parsers.ParseImageName(image) imgName, err := appctypes.SanitizeACIdentifier(repoToPull) if err != nil { return nil, err @@ -929,7 +918,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, _ := parseImageName(img) + repoToPull, _ := parsers.ParseImageName(img) keyring, err := credentialprovider.MakeDockerKeyring(pullSecrets, r.dockerKeyring) if err != nil { return err @@ -955,7 +944,7 @@ func (r *Runtime) PullImage(image kubecontainer.ImageSpec, pullSecrets []api.Sec // TODO(yifan): Searching the image via 'rkt images' might not be the most efficient way. func (r *Runtime) IsImagePresent(image kubecontainer.ImageSpec) (bool, error) { - repoToPull, tag := parseImageName(image.Image) + repoToPull, tag := parsers.ParseImageName(image.Image) // Example output of 'rkt image list --fields=name': // // NAME diff --git a/pkg/util/parsers/parsers.go b/pkg/util/parsers/parsers.go new file mode 100644 index 00000000000..f7c352ef503 --- /dev/null +++ b/pkg/util/parsers/parsers.go @@ -0,0 +1,36 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package parsers + +import ( + "github.com/docker/docker/pkg/parsers" +) + +const ( + 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) { + repoToPull, tag := parsers.ParseRepositoryTag(image) + // If no tag was specified, use the default "latest". + if len(tag) == 0 { + tag = defaultImageTag + } + return repoToPull, tag +} From 5552d7007c2ad13655438ecb64047b8debf469b7 Mon Sep 17 00:00:00 2001 From: Harry Zhang Date: Sat, 14 Nov 2015 05:21:00 +0000 Subject: [PATCH 2/2] Add default when latest not claimed --- docs/user-guide/images.md | 3 +++ pkg/api/v1/defaults.go | 9 ++++----- pkg/kubelet/config/file_test.go | 2 +- pkg/kubelet/config/http_test.go | 4 ++-- pkg/kubelet/dockertools/docker.go | 6 +----- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/docs/user-guide/images.md b/docs/user-guide/images.md index 92d04fb9f96..eddbba6d46a 100644 --- a/docs/user-guide/images.md +++ b/docs/user-guide/images.md @@ -60,6 +60,9 @@ pull an image if it already exists. If you would like to always force a pull you must set a pull image policy of `Always` or specify a `:latest` tag on your image. +If you did not specify tag of your image, it will be assumed as `:latest`, with +pull image policy of `Always` correspondingly. + ## Using a Private Registry Private registries may require keys to read images from them. diff --git a/pkg/api/v1/defaults.go b/pkg/api/v1/defaults.go index 68ce75bb951..88da9c442a6 100644 --- a/pkg/api/v1/defaults.go +++ b/pkg/api/v1/defaults.go @@ -17,11 +17,10 @@ limitations under the License. package v1 import ( - "strings" - "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/util/intstr" + "k8s.io/kubernetes/pkg/util/parsers" ) func addDefaultingFuncs() { @@ -59,10 +58,10 @@ func addDefaultingFuncs() { }, func(obj *Container) { if obj.ImagePullPolicy == "" { - // TODO(dchen1107): Move ParseImageName code to pkg/util - parts := strings.Split(obj.Image, ":") + _, tag := parsers.ParseImageName(obj.Image) // Check image tag - if parts[len(parts)-1] == "latest" { + + if tag == "latest" { obj.ImagePullPolicy = PullAlways } else { obj.ImagePullPolicy = PullIfNotPresent diff --git a/pkg/kubelet/config/file_test.go b/pkg/kubelet/config/file_test.go index 0f2e9ea9ec5..be30c035a88 100644 --- a/pkg/kubelet/config/file_test.go +++ b/pkg/kubelet/config/file_test.go @@ -111,7 +111,7 @@ func TestReadPodsFromFile(t *testing.T) { Name: "image", Image: "test/image", TerminationMessagePath: "/dev/termination-log", - ImagePullPolicy: "IfNotPresent", + ImagePullPolicy: "Always", SecurityContext: securitycontext.ValidSecurityContextWithContainerDefaults()}}, SecurityContext: &api.PodSecurityContext{}, }, diff --git a/pkg/kubelet/config/http_test.go b/pkg/kubelet/config/http_test.go index f257bc3fc68..7a40140c127 100644 --- a/pkg/kubelet/config/http_test.go +++ b/pkg/kubelet/config/http_test.go @@ -199,7 +199,7 @@ func TestExtractPodsFromHTTP(t *testing.T) { }, Spec: api.PodSpec{ NodeName: hostname, - Containers: []api.Container{{Name: "2", Image: "bar", ImagePullPolicy: ""}}, + Containers: []api.Container{{Name: "2", Image: "bar:bartag", ImagePullPolicy: ""}}, SecurityContext: &api.PodSecurityContext{}, }, }, @@ -247,7 +247,7 @@ func TestExtractPodsFromHTTP(t *testing.T) { Containers: []api.Container{{ Name: "2", - Image: "bar", + Image: "bar:bartag", TerminationMessagePath: "/dev/termination-log", ImagePullPolicy: "IfNotPresent", }}, diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index 640742e1842..96235ef2a3f 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -132,12 +132,8 @@ func filterHTTPError(err error, image string) error { } func (p dockerPuller) Pull(image string, secrets []api.Secret) error { - repoToPull, tag := parsers.ParseImageName(image) - // If no tag was specified, use the default "latest". - if len(tag) == 0 { - tag = "latest" - } + repoToPull, tag := parsers.ParseImageName(image) opts := docker.PullImageOptions{ Repository: repoToPull,