diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index 89aae7f7e22..ebe50bc10fd 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -43,7 +43,6 @@ import ( const ( PodInfraContainerName = leaky.PodInfraContainerName DockerPrefix = "docker://" - DockerPullablePrefix = "docker-pullable://" LogSuffix = "log" ext4MaxFileNameLen = 255 ) @@ -67,8 +66,7 @@ type DockerInterface interface { StartContainer(id string) error StopContainer(id string, timeout int) error RemoveContainer(id string, opts dockertypes.ContainerRemoveOptions) error - InspectImageByRef(imageRef string) (*dockertypes.ImageInspect, error) - InspectImageByID(imageID string) (*dockertypes.ImageInspect, error) + InspectImage(image 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) @@ -138,11 +136,7 @@ func filterHTTPError(err error, image string) error { } } -// 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`). +// Check if the inspected image matches what we are looking for 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 @@ -199,43 +193,6 @@ 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 { @@ -289,7 +246,7 @@ func (p dockerPuller) Pull(image string, secrets []api.Secret) error { } func (p dockerPuller) IsImagePresent(image string) (bool, error) { - _, err := p.client.InspectImageByRef(image) + _, err := p.client.InspectImage(image) if err == nil { return true, nil } diff --git a/pkg/kubelet/dockertools/docker_manager.go b/pkg/kubelet/dockertools/docker_manager.go index 438d20d92cf..c8346a382bf 100644 --- a/pkg/kubelet/dockertools/docker_manager.go +++ b/pkg/kubelet/dockertools/docker_manager.go @@ -397,26 +397,11 @@ 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: imageID, + ImageID: DockerPrefix + iResult.Image, ID: kubecontainer.DockerID(id).ContainerID(), ExitCode: iResult.State.ExitCode, CreatedAt: createdAt, @@ -928,7 +913,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.InspectImageByID(image.Image); err == nil && len(inspectImage.RepoTags) > 1 { + if inspectImage, err := dm.client.InspectImage(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 @@ -2429,7 +2414,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.InspectImageByRef(image) + img, err := dm.client.InspectImage(image) if err != nil { return false, err } diff --git a/pkg/kubelet/dockertools/docker_manager_test.go b/pkg/kubelet/dockertools/docker_manager_test.go index 15a6b2f7770..90f1172abd5 100644 --- a/pkg/kubelet/dockertools/docker_manager_test.go +++ b/pkg/kubelet/dockertools/docker_manager_test.go @@ -151,9 +151,6 @@ func createTestDockerManager(fakeHTTPClient *fakeHTTP, fakeDocker *FakeDockerCli fakeHTTPClient, flowcontrol.NewBackOff(time.Second, 300*time.Second)) - // default this to an empty result, so that we never have a nil non-error response from InspectImage - fakeDocker.Image = &dockertypes.ImageInspect{} - return dockerManager, fakeDocker } diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 4516cef7164..745c5cf6dce 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -315,85 +315,6 @@ 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 @@ -693,14 +614,8 @@ type imageTrackingDockerClient struct { imageName string } -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) +func (f *imageTrackingDockerClient) InspectImage(name string) (image *dockertypes.ImageInspect, err error) { + image, err = f.FakeDockerClient.InspectImage(name) f.imageName = name return } diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index 5a6c64164d1..85bea450b0c 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/pkg/kubelet/dockertools/fake_docker_client.go @@ -310,19 +310,9 @@ func (f *FakeDockerClient) InspectContainer(id string) (*dockertypes.ContainerJS return nil, fmt.Errorf("container %q not found", id) } -// InspectImageByRef is a test-spy implementation of DockerInterface.InspectImageByRef. +// InspectImage is a test-spy implementation of DockerInterface.InspectImage. // It adds an entry "inspect" to the internal method call record. -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) { +func (f *FakeDockerClient) InspectImage(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 66de3aecfea..eb15226498a 100644 --- a/pkg/kubelet/dockertools/instrumented_docker.go +++ b/pkg/kubelet/dockertools/instrumented_docker.go @@ -107,20 +107,11 @@ func (in instrumentedDockerInterface) RemoveContainer(id string, opts dockertype return err } -func (in instrumentedDockerInterface) InspectImageByRef(image string) (*dockertypes.ImageInspect, error) { +func (in instrumentedDockerInterface) InspectImage(image string) (*dockertypes.ImageInspect, error) { const operation = "inspect_image" defer recordOperation(operation, time.Now()) - 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) + out, err := in.client.InspectImage(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 32297fa28c2..e6355748c93 100644 --- a/pkg/kubelet/dockertools/kube_docker_client.go +++ b/pkg/kubelet/dockertools/kube_docker_client.go @@ -182,47 +182,25 @@ func (d *kubeDockerClient) RemoveContainer(id string, opts dockertypes.Container return err } -func (d *kubeDockerClient) inspectImageRaw(ref string) (*dockertypes.ImageInspect, error) { +func (d *kubeDockerClient) InspectImage(image string) (*dockertypes.ImageInspect, error) { ctx, cancel := d.getTimeoutContext() defer cancel() - resp, _, err := d.client.ImageInspectWithRaw(ctx, ref, true) + resp, _, err := d.client.ImageInspectWithRaw(ctx, image, true) if ctxErr := contextError(ctx); ctxErr != nil { return nil, ctxErr } if err != nil { if dockerapi.IsErrImageNotFound(err) { - err = imageNotFoundError{ID: ref} + err = imageNotFoundError{ID: image} } 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 deleted file mode 100644 index d81ac51a688..00000000000 --- a/test/e2e_node/image_id_test.go +++ /dev/null @@ -1,66 +0,0 @@ -/* -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 b6a8b93bf40..a6031703661 100644 --- a/test/e2e_node/image_list.go +++ b/test/e2e_node/image_list.go @@ -41,7 +41,6 @@ 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",