From 3b545abf126529c807ecea4f55b86afb7d78d14f Mon Sep 17 00:00:00 2001
From: Antonio Murdaca <runcom@redhat.com>
Date: Wed, 7 Jun 2017 19:54:02 +0200
Subject: [PATCH] image_pull: check image already pulled

This is an optimization of our image pull code path. It's basically
how docker handles pulls as well. Let's be smart and check the image in
pull code path as well.
This also matches docker behavior which first checks whether we're
allowed to actually pull an image before looking into local storage.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
---
 pkg/storage/image.go | 32 ++++++++++++++++++++++++++++++++
 server/image_pull.go | 23 ++++++++++++++++++++---
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/pkg/storage/image.go b/pkg/storage/image.go
index dc1ba341..cdb02e87 100644
--- a/pkg/storage/image.go
+++ b/pkg/storage/image.go
@@ -3,6 +3,7 @@ package storage
 import (
 	"github.com/containers/image/copy"
 	"github.com/containers/image/docker/reference"
+	"github.com/containers/image/image"
 	"github.com/containers/image/signature"
 	istorage "github.com/containers/image/storage"
 	"github.com/containers/image/transports/alltransports"
@@ -38,6 +39,8 @@ type ImageServer interface {
 	// the image server uses to hold images, and is the destination used
 	// when it's asked to pull an image.
 	GetStore() storage.Store
+	// CanPull preliminary checks whether we're allowed to pull an image
+	CanPull(imageName string, sourceCtx *types.SystemContext) (bool, error)
 }
 
 func (svc *imageService) ListImages(filter string) ([]ImageResult, error) {
@@ -116,6 +119,35 @@ func imageSize(img types.Image) *uint64 {
 	return nil
 }
 
+func (svc *imageService) CanPull(imageName string, sourceCtx *types.SystemContext) (bool, error) {
+	if imageName == "" {
+		return false, storage.ErrNotAnImage
+	}
+	srcRef, err := alltransports.ParseImageName(imageName)
+	if err != nil {
+		if svc.defaultTransport == "" {
+			return false, err
+		}
+		srcRef2, err2 := alltransports.ParseImageName(svc.defaultTransport + imageName)
+		if err2 != nil {
+			return false, err
+		}
+		srcRef = srcRef2
+	}
+	rawSource, err := srcRef.NewImageSource(sourceCtx, nil)
+	if err != nil {
+		return false, err
+	}
+	unparsedImage := image.UnparsedFromSource(rawSource)
+	defer unparsedImage.Close()
+	src, err := image.FromUnparsedImage(unparsedImage)
+	if err != nil {
+		return false, err
+	}
+	src.Close()
+	return true, nil
+}
+
 func (svc *imageService) PullImage(systemContext *types.SystemContext, imageName string, options *copy.Options) (types.ImageReference, error) {
 	policy, err := signature.DefaultPolicy(systemContext)
 	if err != nil {
diff --git a/server/image_pull.go b/server/image_pull.go
index b9f1b9f1..b53f4988 100644
--- a/server/image_pull.go
+++ b/server/image_pull.go
@@ -21,6 +21,7 @@ func (s *Server) PullImage(ctx context.Context, req *pb.PullImageRequest) (*pb.P
 	if img != nil {
 		image = img.Image
 	}
+
 	var (
 		username string
 		password string
@@ -36,7 +37,11 @@ func (s *Server) PullImage(ctx context.Context, req *pb.PullImageRequest) (*pb.P
 			}
 		}
 	}
-	options := &copy.Options{}
+	options := &copy.Options{
+		// TODO: we need a way to specify insecure registries like docker
+		//DockerInsecureSkipTLSVerify: true,
+		SourceCtx: &types.SystemContext{},
+	}
 	// a not empty username should be sufficient to decide whether to send auth
 	// or not I guess
 	if username != "" {
@@ -47,8 +52,20 @@ func (s *Server) PullImage(ctx context.Context, req *pb.PullImageRequest) (*pb.P
 			},
 		}
 	}
-	_, err := s.storageImageServer.PullImage(s.imageContext, image, options)
-	if err != nil {
+
+	canPull, err := s.storageImageServer.CanPull(image, options.SourceCtx)
+	if err != nil && !canPull {
+		return nil, err
+	}
+
+	// let's be smart, docker doesn't repull if image already exists.
+	if _, err := s.storageImageServer.ImageStatus(s.imageContext, image); err == nil {
+		return &pb.PullImageResponse{
+			ImageRef: image,
+		}, nil
+	}
+
+	if _, err := s.storageImageServer.PullImage(s.imageContext, image, options); err != nil {
 		return nil, err
 	}
 	resp := &pb.PullImageResponse{