Skip to content

K
k3s
Commit 77371c3b authored by Wojciech Tyczynski's avatar Wojciech Tyczynski Committed by GitHub
Browse files
Options

Revert "Kubelet: Use RepoDigest for ImageID when available"

parent 28a53453
Hide whitespace changes
Inline Side-by-side
Showing with 18 additions and 272 deletions
pkg/kubelet/dockertools/docker.go
View file @ 77371c3b
......@@ -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
}
......
pkg/kubelet/dockertools/docker_manager.go
View file @ 77371c3b
......@@ -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
}
......
pkg/kubelet/dockertools/docker_manager_test.go
View file @ 77371c3b
......@@ -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
}
......
pkg/kubelet/dockertools/docker_test.go
View file @ 77371c3b
......@@ -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
}
......
pkg/kubelet/dockertools/fake_docker_client.go
View file @ 77371c3b
......@@ -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"})
......
pkg/kubelet/dockertools/instrumented_docker.go
View file @ 77371c3b
......@@ -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
}
......
pkg/kubelet/dockertools/kube_docker_client.go
View file @ 77371c3b
......@@ -182,45 +182,23 @@ 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
}
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}
if !matchImageTagOrSHA(resp, image) {
return nil, imageNotFoundError{ID: image}
}
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
return &resp, nil
}
func (d *kubeDockerClient) ImageHistory(id string) ([]dockertypes.ImageHistory, error) {
......
test/e2e_node/image_id_test.go deleted 100644 → 0
View file @ 28a53453
/*
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))
})
})
test/e2e_node/image_list.go
View file @ 77371c3b
......@@ -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",
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment