From aeea6565813c35a457d74efd47f5d586d82f38ea Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Mon, 16 Jan 2017 13:19:44 -0500 Subject: [PATCH] Limit implicit image pulling to the pause image The CRI doesn't expect us to implicitly pull an image if it isn't already present before we're asked to use it to create a container, and the tests no longer depend on us doing so, either. Limit the logic which attempts to pull an image, if it isn't present, to only pulling the configured "pause" image, since our use of that image for running pod sandboxes is an implementation detail that our clients can't be expected to know or care about. Include the name of the image that we didn't pull in the error we return when we don't pull one. Signed-off-by: Nalin Dahyabhai --- pkg/storage/runtime.go | 25 ++++++++++++++++--------- server/server.go | 2 +- test/helpers.bash | 5 +++++ 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/pkg/storage/runtime.go b/pkg/storage/runtime.go index 5347bd48..08c451f5 100644 --- a/pkg/storage/runtime.go +++ b/pkg/storage/runtime.go @@ -3,6 +3,7 @@ package storage import ( "encoding/json" "errors" + "fmt" "time" "github.com/Sirupsen/logrus" @@ -38,7 +39,8 @@ var ( ) type runtimeService struct { - image ImageServer + image ImageServer + pauseImage string } // ContainerInfo wraps a subset of information about a container: its ID and @@ -176,12 +178,7 @@ func (r *runtimeService) createContainerOrPodSandbox(systemContext *types.System } img, err = istorage.Transport.GetStoreImage(r.image.GetStore(), ref) } - if err != nil && err != storage.ErrImageUnknown { - return ContainerInfo{}, err - } - - // Pull the image down if we don't already have it. - if err == storage.ErrImageUnknown { + if img == nil && err == storage.ErrImageUnknown && imageName == r.pauseImage { image := imageID if imageName != "" { image = imageName @@ -200,6 +197,15 @@ func (r *runtimeService) createContainerOrPodSandbox(systemContext *types.System } logrus.Debugf("successfully pulled image %q", image) } + if img == nil && err == storage.ErrImageUnknown { + if imageID == "" { + return ContainerInfo{}, fmt.Errorf("image %q not present in image store", imageName) + } + if imageName == "" { + return ContainerInfo{}, fmt.Errorf("image with ID %q not present in image store", imageID) + } + return ContainerInfo{}, fmt.Errorf("image %q with ID %q not present in image store", imageName, imageID) + } // Pull out a copy of the image's configuration. image, err := ref.NewImage(systemContext) @@ -449,8 +455,9 @@ func (r *runtimeService) GetRunDir(id string) (string, error) { // GetRuntimeService returns a RuntimeServer that uses the passed-in image // service to pull and manage images, and its store to manage containers based // on those images. -func GetRuntimeService(image ImageServer) RuntimeServer { +func GetRuntimeService(image ImageServer, pauseImage string) RuntimeServer { return &runtimeService{ - image: image, + image: image, + pauseImage: pauseImage, } } diff --git a/server/server.go b/server/server.go index b47697ee..979b3ea3 100644 --- a/server/server.go +++ b/server/server.go @@ -465,7 +465,7 @@ func New(config *Config) (*Server, error) { return nil, err } - storageRuntimeService := storage.GetRuntimeService(imageService) + storageRuntimeService := storage.GetRuntimeService(imageService, config.PauseImage) if err != nil { return nil, err } diff --git a/test/helpers.bash b/test/helpers.bash index 58eaee24..f0e2f8e2 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -134,6 +134,11 @@ function start_ocid() { "$OCID_BINARY" --conmon "$CONMON_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNC_BINARY" --root "$TESTDIR/ocid" --runroot "$TESTDIR/ocid-run" --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$OCID_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json config >$OCID_CONFIG "$OCID_BINARY" --debug --config "$OCID_CONFIG" & OCID_PID=$! wait_until_reachable + + run ocic image status --id=redis + if [ "$status" -ne 0 ] ; then + ocic image pull docker://redis:latest + fi } function cleanup_ctrs() {