diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index ebe50bc10fd..89aae7f7e22 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -43,6 +43,7 @@ import ( const ( PodInfraContainerName = leaky.PodInfraContainerName DockerPrefix = "docker://" + DockerPullablePrefix = "docker-pullable://" LogSuffix = "log" ext4MaxFileNameLen = 255 ) @@ -66,7 +67,8 @@ type DockerInterface interface { StartContainer(id string) error StopContainer(id string, timeout int) error RemoveContainer(id string, opts dockertypes.ContainerRemoveOptions) error - InspectImage(image string) (*dockertypes.ImageInspect, error) + InspectImageByRef(imageRef string) (*dockertypes.ImageInspect, error) + InspectImageByID(imageID string) (*dockertypes.ImageInspect, error) ListImages(opts dockertypes.ImageListOptions) ([]dockertypes.Image, error) PullImage(image string, auth dockertypes.AuthConfig, opts dockertypes.ImagePullOptions) error RemoveImage(image string, opts dockertypes.ImageRemoveOptions) ([]dockertypes.ImageDelete, error) @@ -136,7 +138,11 @@ func filterHTTPError(err error, image string) error { } } -// Check if the inspected image matches what we are looking for +// matchImageTagOrSHA checks if the given image specifier is a valid image ref, +// and that it matches the given image. It should fail on things like image IDs +// (config digests) and other digest-only references, but succeed on image names +// (`foo`), tag references (`foo:bar`), and manifest digest references +// (`foo@sha256:xyz`). func matchImageTagOrSHA(inspected dockertypes.ImageInspect, image string) bool { // The image string follows the grammar specified here // https://github.com/docker/distribution/blob/master/reference/reference.go#L4 @@ -193,6 +199,43 @@ func matchImageTagOrSHA(inspected dockertypes.ImageInspect, image string) bool { return false } +// matchImageIDOnly checks that the given image specifier is a digest-only +// reference, and that it matches the given image. +func matchImageIDOnly(inspected dockertypes.ImageInspect, image string) bool { + // If the image ref is literally equal to the inspected image's ID, + // just return true here (this might be the case for Docker 1.9, + // where we won't have a digest for the ID) + if inspected.ID == image { + return true + } + + // Otherwise, we should try actual parsing to be more correct + ref, err := dockerref.Parse(image) + if err != nil { + glog.V(4).Infof("couldn't parse image reference %q: %v", image, err) + return false + } + + digest, isDigested := ref.(dockerref.Digested) + if !isDigested { + glog.V(4).Infof("the image reference %q was not a digest reference") + return false + } + + id, err := dockerdigest.ParseDigest(inspected.ID) + if err != nil { + glog.V(4).Infof("couldn't parse image ID reference %q: %v", id, err) + return false + } + + if digest.Digest().Algorithm().String() == id.Algorithm().String() && digest.Digest().Hex() == id.Hex() { + return true + } + + glog.V(4).Infof("The reference %s does not directly refer to the given image's ID (%q)", image, inspected.ID) + return false +} + func (p dockerPuller) Pull(image string, secrets []api.Secret) error { keyring, err := credentialprovider.MakeDockerKeyring(secrets, p.keyring) if err != nil { @@ -246,7 +289,7 @@ func (p dockerPuller) Pull(image string, secrets []api.Secret) error { } func (p dockerPuller) IsImagePresent(image string) (bool, error) { - _, err := p.client.InspectImage(image) + _, err := p.client.InspectImageByRef(image) if err == nil { return true, nil } diff --git a/pkg/kubelet/dockertools/docker_manager.go b/pkg/kubelet/dockertools/docker_manager.go index c8346a382bf..438d20d92cf 100644 --- a/pkg/kubelet/dockertools/docker_manager.go +++ b/pkg/kubelet/dockertools/docker_manager.go @@ -397,11 +397,26 @@ func (dm *DockerManager) inspectContainer(id string, podName, podNamespace strin parseTimestampError("FinishedAt", iResult.State.FinishedAt) } + // default to the image ID, but try and inspect for the RepoDigests + imageID := DockerPrefix + iResult.Image + imgInspectResult, err := dm.client.InspectImageByID(iResult.Image) + if err != nil { + utilruntime.HandleError(fmt.Errorf("unable to inspect docker image %q while inspecting docker container %q: %v", containerName, iResult.Image, err)) + } else { + if len(imgInspectResult.RepoDigests) > 1 { + glog.V(4).Infof("Container %q had more than one associated RepoDigest (%v), only using the first", containerName, imgInspectResult.RepoDigests) + } + + if len(imgInspectResult.RepoDigests) > 0 { + imageID = DockerPullablePrefix + imgInspectResult.RepoDigests[0] + } + } + status := kubecontainer.ContainerStatus{ Name: containerName, RestartCount: containerInfo.RestartCount, Image: iResult.Config.Image, - ImageID: DockerPrefix + iResult.Image, + ImageID: imageID, ID: kubecontainer.DockerID(id).ContainerID(), ExitCode: iResult.State.ExitCode, CreatedAt: createdAt, @@ -913,7 +928,7 @@ func (dm *DockerManager) IsImagePresent(image kubecontainer.ImageSpec) (bool, er // Removes the specified image. func (dm *DockerManager) RemoveImage(image kubecontainer.ImageSpec) error { // If the image has multiple tags, we need to remove all the tags - if inspectImage, err := dm.client.InspectImage(image.Image); err == nil && len(inspectImage.RepoTags) > 1 { + if inspectImage, err := dm.client.InspectImageByID(image.Image); err == nil && len(inspectImage.RepoTags) > 1 { for _, tag := range inspectImage.RepoTags { if _, err := dm.client.RemoveImage(tag, dockertypes.ImageRemoveOptions{PruneChildren: true}); err != nil { return err @@ -2414,7 +2429,7 @@ func (dm *DockerManager) verifyNonRoot(container *api.Container) error { // or the user is set to root. If there is an error inspecting the image this method will return // false and return the error. func (dm *DockerManager) isImageRoot(image string) (bool, error) { - img, err := dm.client.InspectImage(image) + img, err := dm.client.InspectImageByRef(image) if err != nil { return false, err } diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 745c5cf6dce..4516cef7164 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -315,6 +315,85 @@ func TestMatchImageTagOrSHA(t *testing.T) { } } +func TestMatchImageIDOnly(t *testing.T) { + for i, testCase := range []struct { + Inspected dockertypes.ImageInspect + Image string + Output bool + }{ + // shouldn't match names or tagged names + { + Inspected: dockertypes.ImageInspect{RepoTags: []string{"ubuntu:latest"}}, + Image: "ubuntu", + Output: false, + }, + { + Inspected: dockertypes.ImageInspect{RepoTags: []string{"colemickens/hyperkube-amd64:217.9beff63"}}, + Image: "colemickens/hyperkube-amd64:217.9beff63", + Output: false, + }, + // should match name@digest refs if they refer to the image ID (but only the full ID) + { + Inspected: dockertypes.ImageInspect{ + ID: "sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d", + }, + Image: "myimage@sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d", + Output: true, + }, + { + Inspected: dockertypes.ImageInspect{ + ID: "sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d", + }, + Image: "myimage@sha256:2208f7a29005", + Output: false, + }, + { + Inspected: dockertypes.ImageInspect{ + ID: "sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d", + }, + Image: "myimage@sha256:2208", + Output: false, + }, + // should match when the IDs are literally the same + { + Inspected: dockertypes.ImageInspect{ + ID: "foobar", + }, + Image: "foobar", + Output: true, + }, + // shouldn't match mismatched IDs + { + Inspected: dockertypes.ImageInspect{ + ID: "sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d", + }, + Image: "myimage@sha256:0000f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d", + Output: false, + }, + // shouldn't match invalid IDs or refs + { + Inspected: dockertypes.ImageInspect{ + ID: "sha256:unparseable", + }, + Image: "myimage@sha256:unparseable", + Output: false, + }, + // shouldn't match against repo digests + { + Inspected: dockertypes.ImageInspect{ + ID: "sha256:9bbdf247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227", + RepoDigests: []string{"centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf"}, + }, + Image: "centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf", + Output: false, + }, + } { + match := matchImageIDOnly(testCase.Inspected, testCase.Image) + assert.Equal(t, testCase.Output, match, fmt.Sprintf("%s is not a match (%d)", testCase.Image, i)) + } + +} + func TestPullWithNoSecrets(t *testing.T) { tests := []struct { imageName string @@ -614,8 +693,14 @@ type imageTrackingDockerClient struct { imageName string } -func (f *imageTrackingDockerClient) InspectImage(name string) (image *dockertypes.ImageInspect, err error) { - image, err = f.FakeDockerClient.InspectImage(name) +func (f *imageTrackingDockerClient) InspectImageByID(name string) (image *dockertypes.ImageInspect, err error) { + image, err = f.FakeDockerClient.InspectImageByID(name) + f.imageName = name + return +} + +func (f *imageTrackingDockerClient) InspectImageByRef(name string) (image *dockertypes.ImageInspect, err error) { + image, err = f.FakeDockerClient.InspectImageByRef(name) f.imageName = name return } diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index 85bea450b0c..0f6908c739e 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/pkg/kubelet/dockertools/fake_docker_client.go @@ -86,6 +86,9 @@ func newClientWithVersionAndClock(version, apiVersion string, c clock.Clock) *Fa Errors: make(map[string]error), ContainerMap: make(map[string]*dockertypes.ContainerJSON), Clock: c, + + // default this to an empty result, so that we never have a nil non-error response from InspectImage + Image: &dockertypes.ImageInspect{}, } } @@ -310,9 +313,19 @@ func (f *FakeDockerClient) InspectContainer(id string) (*dockertypes.ContainerJS return nil, fmt.Errorf("container %q not found", id) } -// InspectImage is a test-spy implementation of DockerInterface.InspectImage. +// InspectImageByRef is a test-spy implementation of DockerInterface.InspectImageByRef. // It adds an entry "inspect" to the internal method call record. -func (f *FakeDockerClient) InspectImage(name string) (*dockertypes.ImageInspect, error) { +func (f *FakeDockerClient) InspectImageByRef(name string) (*dockertypes.ImageInspect, error) { + f.Lock() + defer f.Unlock() + f.called = append(f.called, calledDetail{name: "inspect_image"}) + err := f.popError("inspect_image") + return f.Image, err +} + +// InspectImageByID is a test-spy implementation of DockerInterface.InspectImageByID. +// It adds an entry "inspect" to the internal method call record. +func (f *FakeDockerClient) InspectImageByID(name string) (*dockertypes.ImageInspect, error) { f.Lock() defer f.Unlock() f.called = append(f.called, calledDetail{name: "inspect_image"}) diff --git a/pkg/kubelet/dockertools/instrumented_docker.go b/pkg/kubelet/dockertools/instrumented_docker.go index eb15226498a..66de3aecfea 100644 --- a/pkg/kubelet/dockertools/instrumented_docker.go +++ b/pkg/kubelet/dockertools/instrumented_docker.go @@ -107,11 +107,20 @@ func (in instrumentedDockerInterface) RemoveContainer(id string, opts dockertype return err } -func (in instrumentedDockerInterface) InspectImage(image string) (*dockertypes.ImageInspect, error) { +func (in instrumentedDockerInterface) InspectImageByRef(image string) (*dockertypes.ImageInspect, error) { const operation = "inspect_image" defer recordOperation(operation, time.Now()) - out, err := in.client.InspectImage(image) + out, err := in.client.InspectImageByRef(image) + recordError(operation, err) + return out, err +} + +func (in instrumentedDockerInterface) InspectImageByID(image string) (*dockertypes.ImageInspect, error) { + const operation = "inspect_image" + defer recordOperation(operation, time.Now()) + + out, err := in.client.InspectImageByID(image) recordError(operation, err) return out, err } diff --git a/pkg/kubelet/dockertools/kube_docker_client.go b/pkg/kubelet/dockertools/kube_docker_client.go index e6355748c93..32297fa28c2 100644 --- a/pkg/kubelet/dockertools/kube_docker_client.go +++ b/pkg/kubelet/dockertools/kube_docker_client.go @@ -182,25 +182,47 @@ func (d *kubeDockerClient) RemoveContainer(id string, opts dockertypes.Container return err } -func (d *kubeDockerClient) InspectImage(image string) (*dockertypes.ImageInspect, error) { +func (d *kubeDockerClient) inspectImageRaw(ref string) (*dockertypes.ImageInspect, error) { ctx, cancel := d.getTimeoutContext() defer cancel() - resp, _, err := d.client.ImageInspectWithRaw(ctx, image, true) + resp, _, err := d.client.ImageInspectWithRaw(ctx, ref, true) if ctxErr := contextError(ctx); ctxErr != nil { return nil, ctxErr } if err != nil { if dockerapi.IsErrImageNotFound(err) { - err = imageNotFoundError{ID: image} + err = imageNotFoundError{ID: ref} } return nil, err } - if !matchImageTagOrSHA(resp, image) { - return nil, imageNotFoundError{ID: image} - } + return &resp, nil } +func (d *kubeDockerClient) InspectImageByID(imageID string) (*dockertypes.ImageInspect, error) { + resp, err := d.inspectImageRaw(imageID) + if err != nil { + return nil, err + } + + if !matchImageIDOnly(*resp, imageID) { + return nil, imageNotFoundError{ID: imageID} + } + return resp, nil +} + +func (d *kubeDockerClient) InspectImageByRef(imageRef string) (*dockertypes.ImageInspect, error) { + resp, err := d.inspectImageRaw(imageRef) + if err != nil { + return nil, err + } + + if !matchImageTagOrSHA(*resp, imageRef) { + return nil, imageNotFoundError{ID: imageRef} + } + return resp, nil +} + func (d *kubeDockerClient) ImageHistory(id string) ([]dockertypes.ImageHistory, error) { ctx, cancel := d.getTimeoutContext() defer cancel() diff --git a/test/e2e_node/image_id_test.go b/test/e2e_node/image_id_test.go new file mode 100644 index 00000000000..d81ac51a688 --- /dev/null +++ b/test/e2e_node/image_id_test.go @@ -0,0 +1,66 @@ +/* +Copyright 2016 The Kubernetes Authors. + +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 e2e_node + +import ( + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/kubelet/dockertools" + "k8s.io/kubernetes/test/e2e/framework" + + "github.com/davecgh/go-spew/spew" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = framework.KubeDescribe("ImageID", func() { + + busyBoxImage := "gcr.io/google_containers/busybox@sha256:4bdd623e848417d96127e16037743f0cd8b528c026e9175e22a84f639eca58ff" + + f := framework.NewDefaultFramework("image-id-test") + + It("should be set to the manifest digest (from RepoDigests) when available", func() { + podDesc := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "pod-with-repodigest", + }, + Spec: api.PodSpec{ + Containers: []api.Container{{ + Name: "test", + Image: busyBoxImage, + Command: []string{"sh"}, + }}, + RestartPolicy: api.RestartPolicyNever, + }, + } + + pod := f.PodClient().Create(podDesc) + + framework.ExpectNoError(framework.WaitTimeoutForPodNoLongerRunningInNamespace( + f.Client, pod.Name, f.Namespace.Name, "", framework.PodStartTimeout)) + runningPod, err := f.PodClient().Get(pod.Name) + framework.ExpectNoError(err) + + status := runningPod.Status + + if len(status.ContainerStatuses) == 0 { + framework.Failf("Unexpected pod status; %s", spew.Sdump(status)) + return + } + + Expect(status.ContainerStatuses[0].ImageID).To(Equal(dockertools.DockerPullablePrefix + busyBoxImage)) + }) +}) diff --git a/test/e2e_node/image_list.go b/test/e2e_node/image_list.go index a6031703661..b6a8b93bf40 100644 --- a/test/e2e_node/image_list.go +++ b/test/e2e_node/image_list.go @@ -41,6 +41,7 @@ var NodeImageWhiteList = sets.NewString( "google/cadvisor:latest", "gcr.io/google-containers/stress:v1", "gcr.io/google_containers/busybox:1.24", + "gcr.io/google_containers/busybox@sha256:4bdd623e848417d96127e16037743f0cd8b528c026e9175e22a84f639eca58ff", "gcr.io/google_containers/nginx-slim:0.7", "gcr.io/google_containers/serve_hostname:v1.4", "gcr.io/google_containers/netexec:1.7",