From 29a063e62edc4fe517609fcfe0e4a3d4effa7a75 Mon Sep 17 00:00:00 2001 From: Random-Liu Date: Sun, 26 Feb 2017 20:50:20 -0800 Subject: [PATCH] Check infra container image existence before pulling. --- pkg/kubelet/dockershim/docker_sandbox.go | 5 +-- pkg/kubelet/dockershim/helpers.go | 16 ++++++++++ pkg/kubelet/dockershim/helpers_test.go | 31 +++++++++++++++++++ pkg/kubelet/dockertools/docker.go | 2 +- pkg/kubelet/dockertools/docker_manager.go | 9 +++--- pkg/kubelet/dockertools/kube_docker_client.go | 15 ++++----- 6 files changed, 64 insertions(+), 14 deletions(-) diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go index b1e0e1dfbb7..323317c1508 100644 --- a/pkg/kubelet/dockershim/docker_sandbox.go +++ b/pkg/kubelet/dockershim/docker_sandbox.go @@ -61,8 +61,9 @@ func (ds *dockerService) RunPodSandbox(config *runtimeapi.PodSandboxConfig) (str // NOTE: To use a custom sandbox image in a private repository, users need to configure the nodes with credentials properly. // see: http://kubernetes.io/docs/user-guide/images/#configuring-nodes-to-authenticate-to-a-private-repository - if err := ds.client.PullImage(image, dockertypes.AuthConfig{}, dockertypes.ImagePullOptions{}); err != nil { - return "", fmt.Errorf("unable to pull image for the sandbox container: %v", err) + // Only pull sandbox image when it's not present - v1.PullIfNotPresent. + if err := ensureSandboxImageExists(ds.client, image); err != nil { + return "", err } // Step 2: Create the sandbox container. diff --git a/pkg/kubelet/dockershim/helpers.go b/pkg/kubelet/dockershim/helpers.go index fe0f9cea184..b578ee3386e 100644 --- a/pkg/kubelet/dockershim/helpers.go +++ b/pkg/kubelet/dockershim/helpers.go @@ -320,3 +320,19 @@ func getSecurityOptSeparator(v *semver.Version) rune { return dockertools.SecurityOptSeparatorNew } } + +// ensureSandboxImageExists pulls the sandbox image when it's not present. +func ensureSandboxImageExists(client dockertools.DockerInterface, image string) error { + _, err := client.InspectImageByRef(image) + if err == nil { + return nil + } + if !dockertools.IsImageNotFoundError(err) { + return fmt.Errorf("failed to inspect sandbox image %q: %v", image, err) + } + err = client.PullImage(image, dockertypes.AuthConfig{}, dockertypes.ImagePullOptions{}) + if err != nil { + return fmt.Errorf("unable to pull sandbox image %q: %v", image, err) + } + return nil +} diff --git a/pkg/kubelet/dockershim/helpers_test.go b/pkg/kubelet/dockershim/helpers_test.go index 709b5c21449..56cae261e7a 100644 --- a/pkg/kubelet/dockershim/helpers_test.go +++ b/pkg/kubelet/dockershim/helpers_test.go @@ -17,6 +17,7 @@ limitations under the License. package dockershim import ( + "fmt" "testing" "github.com/blang/semver" @@ -25,6 +26,7 @@ import ( "k8s.io/kubernetes/pkg/api/v1" runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" + "k8s.io/kubernetes/pkg/kubelet/dockertools" "k8s.io/kubernetes/pkg/security/apparmor" ) @@ -261,3 +263,32 @@ func TestGetSecurityOptSeparator(t *testing.T) { assert.Equal(t, test.expected, actual, c) } } + +func TestEnsureSandboxImageExists(t *testing.T) { + for desc, test := range map[string]struct { + inject error + calls []string + err bool + }{ + "should not pull image when it already exists": { + inject: nil, + calls: []string{"inspect_image"}, + }, + "should pull image when it doesn't exist": { + inject: dockertools.ImageNotFoundError{ID: "image_id"}, + calls: []string{"inspect_image", "pull"}, + }, + "should return error when inspect image fails": { + inject: fmt.Errorf("arbitrary error"), + calls: []string{"inspect_image"}, + err: true, + }, + } { + t.Logf("TestCase: %q", desc) + _, fakeDocker, _ := newTestDockerService() + fakeDocker.InjectError("inspect_image", test.inject) + err := ensureSandboxImageExists(fakeDocker, "gcr.io/test/image") + assert.NoError(t, fakeDocker.AssertCalls(test.calls)) + assert.Equal(t, test.err, err != nil) + } +} diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index db4f82ddeb1..10ede873a5c 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -290,7 +290,7 @@ func (p dockerPuller) GetImageRef(image string) (string, error) { } return imageRef, nil } - if _, ok := err.(imageNotFoundError); ok { + if IsImageNotFoundError(err) { return "", nil } return "", err diff --git a/pkg/kubelet/dockertools/docker_manager.go b/pkg/kubelet/dockertools/docker_manager.go index e97f0777c11..88f453729f0 100644 --- a/pkg/kubelet/dockertools/docker_manager.go +++ b/pkg/kubelet/dockertools/docker_manager.go @@ -405,6 +405,7 @@ func (dm *DockerManager) inspectContainer(id string, podName, podNamespace strin // default to the image ID, but try and inspect for the RepoDigests imageID := DockerPrefix + iResult.Image + imageName := iResult.Config.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", iResult.Image, containerName, err)) @@ -416,12 +417,12 @@ func (dm *DockerManager) inspectContainer(id string, podName, podNamespace strin if len(imgInspectResult.RepoDigests) > 0 { imageID = DockerPullablePrefix + imgInspectResult.RepoDigests[0] } + + if len(imgInspectResult.RepoTags) > 0 { + imageName = imgInspectResult.RepoTags[0] + } } - imageName := iResult.Config.Image - if len(imgInspectResult.RepoTags) > 0 { - imageName = imgInspectResult.RepoTags[0] - } status := kubecontainer.ContainerStatus{ Name: containerName, RestartCount: containerInfo.RestartCount, diff --git a/pkg/kubelet/dockertools/kube_docker_client.go b/pkg/kubelet/dockertools/kube_docker_client.go index 37533789159..20e2acaab94 100644 --- a/pkg/kubelet/dockertools/kube_docker_client.go +++ b/pkg/kubelet/dockertools/kube_docker_client.go @@ -187,7 +187,7 @@ func (d *kubeDockerClient) inspectImageRaw(ref string) (*dockertypes.ImageInspec } if err != nil { if dockerapi.IsErrImageNotFound(err) { - err = imageNotFoundError{ID: ref} + err = ImageNotFoundError{ID: ref} } return nil, err } @@ -202,7 +202,7 @@ func (d *kubeDockerClient) InspectImageByID(imageID string) (*dockertypes.ImageI } if !matchImageIDOnly(*resp, imageID) { - return nil, imageNotFoundError{ID: imageID} + return nil, ImageNotFoundError{ID: imageID} } return resp, nil } @@ -214,7 +214,7 @@ func (d *kubeDockerClient) InspectImageByRef(imageRef string) (*dockertypes.Imag } if !matchImageTagOrSHA(*resp, imageRef) { - return nil, imageNotFoundError{ID: imageRef} + return nil, ImageNotFoundError{ID: imageRef} } return resp, nil } @@ -613,18 +613,19 @@ func IsContainerNotFoundError(err error) bool { return containerNotFoundErrorRegx.MatchString(err.Error()) } -// imageNotFoundError is the error returned by InspectImage when image not found. -type imageNotFoundError struct { +// ImageNotFoundError is the error returned by InspectImage when image not found. +// Expose this to inject error in dockershim for testing. +type ImageNotFoundError struct { ID string } -func (e imageNotFoundError) Error() string { +func (e ImageNotFoundError) Error() string { return fmt.Sprintf("no such image: %q", e.ID) } // IsImageNotFoundError checks whether the error is image not found error. This is exposed // to share with dockershim. func IsImageNotFoundError(err error) bool { - _, ok := err.(imageNotFoundError) + _, ok := err.(ImageNotFoundError) return ok }