From 520b99cdd5e4af7e5740a929e0b55b38b7f02963 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Mon, 29 Jan 2018 12:21:29 +0100 Subject: [PATCH] pkg: kubelet: do not assume anything about images names This patch fixes a regression introduced by https://github.com/kubernetes/kubernetes/pull/51751 in the CRI interface. That commit actually changed a unit test where we were previously *not* assuming anything about an image name. Before that commit, if you send the image "busybox" through the CRI, the container runtime receives "busybox". After that patch the container runtime gets "docker.io/library/busybox". While that may be correct for the internal kube dockershim, in the CRI we must not assume anything about image names. The ImageSpec is not providing any spec around the image so the container runtime should just get the raw image name from the pod spec. Every container runtime can handle image names the way it wants. The "docker.io" namespace is not at all "standard", CRI-O is not following what the docker UI say since that's the docker UI. We should not focus the CRI on wrong UI design, especially around a default namespace. ImageSpec is not standardized yet: https://github.com/kubernetes/kubernetes/issues/46255 and https://github.com/kubernetes/kubernetes/issues/7203 This is something which should land in 1.9 as well since the regression is from 1.8. Signed-off-by: Antonio Murdaca --- pkg/kubelet/images/image_manager.go | 11 ++++++----- pkg/kubelet/images/image_manager_test.go | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/kubelet/images/image_manager.go b/pkg/kubelet/images/image_manager.go index 04652da0403..413df5a727d 100644 --- a/pkg/kubelet/images/image_manager.go +++ b/pkg/kubelet/images/image_manager.go @@ -154,11 +154,12 @@ func applyDefaultImageTag(image string) (string, error) { _, 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() + // we just concatenate the image name with the default tag here instead + // of using dockerref.WithTag(named, ...) because that would cause the + // image to be fully qualified as docker.io/$name if it's a short name + // (e.g. just busybox). We don't want that to happen to keep the CRI + // agnostic wrt image names and default hostnames. + image = image + ":" + parsers.DefaultImageTag } return image, nil } diff --git a/pkg/kubelet/images/image_manager_test.go b/pkg/kubelet/images/image_manager_test.go index 46231172c69..1d79203e1cf 100644 --- a/pkg/kubelet/images/image_manager_test.go +++ b/pkg/kubelet/images/image_manager_test.go @@ -125,7 +125,7 @@ func pullerTestEnv(c pullerTestCase, serialized bool) (puller ImageManager, fake fakeRuntime = &ctest.FakeRuntime{} fakeRecorder := &record.FakeRecorder{} - fakeRuntime.ImageList = []Image{{ID: "docker.io/library/present_image:latest"}} + fakeRuntime.ImageList = []Image{{ID: "present_image:latest"}} fakeRuntime.Err = c.pullerErr fakeRuntime.InspectErr = c.inspectErr @@ -188,7 +188,7 @@ func TestApplyDefaultImageTag(t *testing.T) { Input string Output string }{ - {Input: "root", Output: "docker.io/library/root:latest"}, + {Input: "root", Output: "root:latest"}, {Input: "root:tag", Output: "root:tag"}, {Input: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Output: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, } {