Merge pull request #58955 from runcom/fix-cri-image-spec

Automatic merge from submit-queue (batch tested with PRs 58955, 58968, 58971, 58963, 58298). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

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.
Image name normalization is a Docker implementation detail around short images names, not the CRI. 

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 <runcom@redhat.com>



**What this PR does / why we need it**:

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
Fix regression in the CRI: do not add a default hostname on short image names
```
This commit is contained in:
Kubernetes Submit Queue 2018-01-29 13:48:39 -08:00 committed by GitHub
commit da601bc72a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 8 additions and 7 deletions

View File

@ -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
}

View File

@ -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"},
} {