Merge pull request #574 from runcom/smarter-pull

image_pull: check image already pulled

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 {

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{