diff --git a/pkg/kubelet/dockershim/docker_image.go b/pkg/kubelet/dockershim/docker_image.go index 8bdd24bdf9f..fc0dc76eef7 100644 --- a/pkg/kubelet/dockershim/docker_image.go +++ b/pkg/kubelet/dockershim/docker_image.go @@ -65,7 +65,6 @@ func (ds *dockerService) ImageStatus(image *runtimeApi.ImageSpec) (*runtimeApi.I // PullImage pulls an image with authentication config. func (ds *dockerService) PullImage(image *runtimeApi.ImageSpec, auth *runtimeApi.AuthConfig) error { - // TODO: add default tags for images or should this be done by kubelet? return ds.client.PullImage(image.GetImage(), dockertypes.AuthConfig{ Username: auth.GetUsername(), diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index 6afee7164e5..ebe50bc10fd 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -38,7 +38,6 @@ import ( "k8s.io/kubernetes/pkg/kubelet/leaky" "k8s.io/kubernetes/pkg/types" utilerrors "k8s.io/kubernetes/pkg/util/errors" - "k8s.io/kubernetes/pkg/util/parsers" ) const ( @@ -194,32 +193,7 @@ func matchImageTagOrSHA(inspected dockertypes.ImageInspect, image string) bool { return false } -// 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 the image contains no tag or digest, a default tag should be applied. - image, err := applyDefaultImageTag(image) - if err != nil { - return err - } - keyring, err := credentialprovider.MakeDockerKeyring(secrets, p.keyring) if err != nil { return err diff --git a/pkg/kubelet/dockertools/docker_manager_test.go b/pkg/kubelet/dockertools/docker_manager_test.go index ffd7c00814d..90f1172abd5 100644 --- a/pkg/kubelet/dockertools/docker_manager_test.go +++ b/pkg/kubelet/dockertools/docker_manager_test.go @@ -110,6 +110,16 @@ func (f *fakeRuntimeHelper) GetExtraSupplementalGroupsForPod(pod *api.Pod) []int return nil } +type fakeImageManager struct{} + +func newFakeImageManager() images.ImageManager { + return &fakeImageManager{} +} + +func (m *fakeImageManager) EnsureImageExists(pod *api.Pod, container *api.Container, pullSecrets []api.Secret) (error, string) { + return nil, "" +} + func createTestDockerManager(fakeHTTPClient *fakeHTTP, fakeDocker *FakeDockerClient) (*DockerManager, *FakeDockerClient) { if fakeHTTPClient == nil { fakeHTTPClient = &fakeHTTP{} @@ -144,17 +154,26 @@ func createTestDockerManager(fakeHTTPClient *fakeHTTP, fakeDocker *FakeDockerCli return dockerManager, fakeDocker } +func createTestDockerManagerWithFakeImageManager(fakeHTTPClient *fakeHTTP, fakeDocker *FakeDockerClient) (*DockerManager, *FakeDockerClient) { + dm, fd := createTestDockerManager(fakeHTTPClient, fakeDocker) + dm.imagePuller = newFakeImageManager() + return dm, fd +} + +func newTestDockerManagerWithRealImageManager() (*DockerManager, *FakeDockerClient) { + return createTestDockerManager(nil, nil) +} func newTestDockerManagerWithHTTPClient(fakeHTTPClient *fakeHTTP) (*DockerManager, *FakeDockerClient) { - return createTestDockerManager(fakeHTTPClient, nil) + return createTestDockerManagerWithFakeImageManager(fakeHTTPClient, nil) } func newTestDockerManagerWithVersion(version, apiVersion string) (*DockerManager, *FakeDockerClient) { fakeDocker := NewFakeDockerClientWithVersion(version, apiVersion) - return createTestDockerManager(nil, fakeDocker) + return createTestDockerManagerWithFakeImageManager(nil, fakeDocker) } func newTestDockerManager() (*DockerManager, *FakeDockerClient) { - return createTestDockerManager(nil, nil) + return createTestDockerManagerWithFakeImageManager(nil, nil) } func matchString(t *testing.T, pattern, str string) bool { @@ -617,14 +636,14 @@ func TestSyncPodCreateNetAndContainer(t *testing.T) { } func TestSyncPodCreatesNetAndContainerPullsImage(t *testing.T) { - dm, fakeDocker := newTestDockerManager() - dm.podInfraContainerImage = "pod_infra_image" + dm, fakeDocker := newTestDockerManagerWithRealImageManager() + dm.podInfraContainerImage = "foo/infra_image:v1" puller := dm.dockerPuller.(*FakeDockerPuller) puller.HasImages = []string{} - dm.podInfraContainerImage = "pod_infra_image" + dm.podInfraContainerImage = "foo/infra_image:v1" pod := makePod("foo", &api.PodSpec{ Containers: []api.Container{ - {Name: "bar", Image: "something", ImagePullPolicy: "IfNotPresent"}, + {Name: "bar", Image: "foo/something:v0", ImagePullPolicy: "IfNotPresent"}, }, }) @@ -639,7 +658,7 @@ func TestSyncPodCreatesNetAndContainerPullsImage(t *testing.T) { fakeDocker.Lock() - if !reflect.DeepEqual(puller.ImagesPulled, []string{"pod_infra_image", "something"}) { + if !reflect.DeepEqual(puller.ImagesPulled, []string{"foo/infra_image:v1", "foo/something:v0"}) { t.Errorf("unexpected pulled containers: %v", puller.ImagesPulled) } @@ -1478,18 +1497,18 @@ func TestGetIPCMode(t *testing.T) { } func TestSyncPodWithPullPolicy(t *testing.T) { - dm, fakeDocker := newTestDockerManager() + dm, fakeDocker := newTestDockerManagerWithRealImageManager() puller := dm.dockerPuller.(*FakeDockerPuller) - puller.HasImages = []string{"existing_one", "want:latest"} - dm.podInfraContainerImage = "pod_infra_image" + puller.HasImages = []string{"foo/existing_one:v1", "foo/want:latest"} + dm.podInfraContainerImage = "foo/infra_image:v1" pod := makePod("foo", &api.PodSpec{ Containers: []api.Container{ - {Name: "bar", Image: "pull_always_image", ImagePullPolicy: api.PullAlways}, - {Name: "bar2", Image: "pull_if_not_present_image", ImagePullPolicy: api.PullIfNotPresent}, - {Name: "bar3", Image: "existing_one", ImagePullPolicy: api.PullIfNotPresent}, - {Name: "bar4", Image: "want:latest", ImagePullPolicy: api.PullIfNotPresent}, - {Name: "bar5", Image: "pull_never_image", ImagePullPolicy: api.PullNever}, + {Name: "bar", Image: "foo/pull_always_image:v1", ImagePullPolicy: api.PullAlways}, + {Name: "bar2", Image: "foo/pull_if_not_present_image:v1", ImagePullPolicy: api.PullIfNotPresent}, + {Name: "bar3", Image: "foo/existing_one:v1", ImagePullPolicy: api.PullIfNotPresent}, + {Name: "bar4", Image: "foo/want:latest", ImagePullPolicy: api.PullIfNotPresent}, + {Name: "bar5", Image: "foo/pull_never_image:v1", ImagePullPolicy: api.PullNever}, }, }) @@ -1503,7 +1522,7 @@ func TestSyncPodWithPullPolicy(t *testing.T) { {kubecontainer.StartContainer, "bar3", nil, ""}, {kubecontainer.StartContainer, "bar4", nil, ""}, {kubecontainer.StartContainer, "bar5", images.ErrImageNeverPull, - "Container image \"pull_never_image\" is not present with pull policy of Never"}, + "Container image \"foo/pull_never_image:v1\" is not present with pull policy of Never"}, } result := runSyncPod(t, dm, fakeDocker, pod, nil, true) @@ -1514,7 +1533,7 @@ func TestSyncPodWithPullPolicy(t *testing.T) { pulledImageSorted := puller.ImagesPulled[:] sort.Strings(pulledImageSorted) - assert.Equal(t, []string{"pod_infra_image", "pull_always_image", "pull_if_not_present_image"}, pulledImageSorted) + assert.Equal(t, []string{"foo/infra_image:v1", "foo/pull_always_image:v1", "foo/pull_if_not_present_image:v1"}, pulledImageSorted) if len(fakeDocker.Created) != 5 { t.Errorf("unexpected containers created %v", fakeDocker.Created) @@ -1533,19 +1552,19 @@ func TestSyncPodWithFailure(t *testing.T) { expected []*kubecontainer.SyncResult }{ "PullImageFailure": { - api.Container{Name: "bar", Image: "realImage", ImagePullPolicy: api.PullAlways}, + api.Container{Name: "bar", Image: "foo/real_image:v1", ImagePullPolicy: api.PullAlways}, map[string]error{}, []error{fmt.Errorf("can't pull image")}, []*kubecontainer.SyncResult{{kubecontainer.StartContainer, "bar", images.ErrImagePull, "can't pull image"}}, }, "CreateContainerFailure": { - api.Container{Name: "bar", Image: "alreadyPresent"}, + api.Container{Name: "bar", Image: "foo/already_present:v2"}, map[string]error{"create": fmt.Errorf("can't create container")}, []error{}, []*kubecontainer.SyncResult{{kubecontainer.StartContainer, "bar", kubecontainer.ErrRunContainer, "can't create container"}}, }, "StartContainerFailure": { - api.Container{Name: "bar", Image: "alreadyPresent"}, + api.Container{Name: "bar", Image: "foo/already_present:v2"}, map[string]error{"start": fmt.Errorf("can't start container")}, []error{}, []*kubecontainer.SyncResult{{kubecontainer.StartContainer, "bar", kubecontainer.ErrRunContainer, "can't start container"}}, @@ -1553,7 +1572,7 @@ func TestSyncPodWithFailure(t *testing.T) { } for _, test := range tests { - dm, fakeDocker := newTestDockerManager() + dm, fakeDocker := newTestDockerManagerWithRealImageManager() puller := dm.dockerPuller.(*FakeDockerPuller) puller.HasImages = []string{test.container.Image} // Pretend that the pod infra container has already been created, so that diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 84cbde8e07c..745c5cf6dce 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -315,34 +315,16 @@ func TestMatchImageTagOrSHA(t *testing.T) { } } -func TestApplyDefaultImageTag(t *testing.T) { - for _, testCase := range []struct { - Input string - Output string - }{ - {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("applyDefaultTag(%s) failed: %v", testCase.Input, err) - } else if image != testCase.Output { - t.Errorf("Expected image reference: %q, got %q", testCase.Output, image) - } - } -} - func TestPullWithNoSecrets(t *testing.T) { tests := []struct { imageName string expectedImage string }{ - {"ubuntu", "ubuntu:latest using {}"}, + {"ubuntu", "ubuntu using {}"}, {"ubuntu:2342", "ubuntu:2342 using {}"}, {"ubuntu:latest", "ubuntu:latest using {}"}, {"foo/bar:445566", "foo/bar:445566 using {}"}, - {"registry.example.com:5000/foobar", "registry.example.com:5000/foobar:latest using {}"}, + {"registry.example.com:5000/foobar", "registry.example.com:5000/foobar using {}"}, {"registry.example.com:5000/foobar:5342", "registry.example.com:5000/foobar:5342 using {}"}, {"registry.example.com:5000/foobar:latest", "registry.example.com:5000/foobar:latest using {}"}, } @@ -430,7 +412,7 @@ func TestPullWithSecrets(t *testing.T) { "ubuntu", []api.Secret{}, credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{}), - []string{"ubuntu:latest using {}"}, + []string{"ubuntu using {}"}, }, "default keyring secrets": { "ubuntu", @@ -438,7 +420,7 @@ func TestPullWithSecrets(t *testing.T) { credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{ "index.docker.io/v1/": {Username: "built-in", Password: "password", Email: "email", Provider: nil}, }), - []string{`ubuntu:latest using {"username":"built-in","password":"password","email":"email"}`}, + []string{`ubuntu using {"username":"built-in","password":"password","email":"email"}`}, }, "default keyring secrets unused": { "ubuntu", @@ -446,7 +428,7 @@ func TestPullWithSecrets(t *testing.T) { credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{ "extraneous": {Username: "built-in", Password: "password", Email: "email", Provider: nil}, }), - []string{`ubuntu:latest using {}`}, + []string{`ubuntu using {}`}, }, "builtin keyring secrets, but use passed": { "ubuntu", @@ -454,7 +436,7 @@ func TestPullWithSecrets(t *testing.T) { credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{ "index.docker.io/v1/": {Username: "built-in", Password: "password", Email: "email", Provider: nil}, }), - []string{`ubuntu:latest using {"username":"passed-user","password":"passed-password","email":"passed-email"}`}, + []string{`ubuntu using {"username":"passed-user","password":"passed-password","email":"passed-email"}`}, }, "builtin keyring secrets, but use passed with new docker config": { "ubuntu", @@ -462,7 +444,7 @@ func TestPullWithSecrets(t *testing.T) { credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{ "index.docker.io/v1/": {Username: "built-in", Password: "password", Email: "email", Provider: nil}, }), - []string{`ubuntu:latest using {"username":"passed-user","password":"passed-password","email":"passed-email"}`}, + []string{`ubuntu using {"username":"passed-user","password":"passed-password","email":"passed-email"}`}, }, } for i, test := range tests { diff --git a/pkg/kubelet/images/image_manager.go b/pkg/kubelet/images/image_manager.go index cf08f946cc8..02ea5b4267a 100644 --- a/pkg/kubelet/images/image_manager.go +++ b/pkg/kubelet/images/image_manager.go @@ -19,12 +19,14 @@ package images import ( "fmt" + dockerref "github.com/docker/distribution/reference" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/record" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/events" "k8s.io/kubernetes/pkg/util/flowcontrol" + "k8s.io/kubernetes/pkg/util/parsers" ) // imageManager provides the functionalities for image pulling. @@ -87,7 +89,15 @@ func (m *imageManager) EnsureImageExists(pod *api.Pod, container *api.Container, glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err) } - spec := kubecontainer.ImageSpec{Image: container.Image} + // If the image contains no tag or digest, a default tag should be applied. + image, err := applyDefaultImageTag(container.Image) + if err != nil { + msg := fmt.Sprintf("Failed to apply default image tag %q: %v", container.Image, err) + m.logIt(ref, api.EventTypeWarning, events.FailedToInspectImage, logPrefix, msg, glog.Warning) + return ErrInvalidImageName, msg + } + + spec := kubecontainer.ImageSpec{Image: image} present, err := m.imageService.IsImagePresent(spec) if err != nil { msg := fmt.Sprintf("Failed to inspect image %q: %v", container.Image, err) @@ -130,3 +140,22 @@ func (m *imageManager) EnsureImageExists(pod *api.Pod, container *api.Container, m.backOff.GC() return nil, "" } + +// 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 +} diff --git a/pkg/kubelet/images/image_manager_test.go b/pkg/kubelet/images/image_manager_test.go index f6a6073f773..7ff7137a627 100644 --- a/pkg/kubelet/images/image_manager_test.go +++ b/pkg/kubelet/images/image_manager_test.go @@ -211,3 +211,21 @@ func TestSerializedPuller(t *testing.T) { } } } + +func TestApplyDefaultImageTag(t *testing.T) { + for _, testCase := range []struct { + Input string + Output string + }{ + {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("applyDefaultImageTag(%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/images/types.go b/pkg/kubelet/images/types.go index ab16bc69767..1f81d947e17 100644 --- a/pkg/kubelet/images/types.go +++ b/pkg/kubelet/images/types.go @@ -37,6 +37,9 @@ var ( // Get http error when pulling image from registry RegistryUnavailable = errors.New("RegistryUnavailable") + + // Unable to parse the image name. + ErrInvalidImageName = errors.New("InvalidImageName") ) // ImageManager provides an interface to manage the lifecycle of images.