From 75af17946d94bc95ba519be5c2b6fc6f1adde4d6 Mon Sep 17 00:00:00 2001
From: Wei Fu <fuweid89@gmail.com>
Date: Mon, 20 Jan 2020 14:11:53 +0800
Subject: [PATCH] Pull: create image record after blobs download

When pull image with unpack option, the fetch action will defer blobs
download until unpack. If create image record in ImageService before
blobs download, the following requests to use image will fail because
there is still missing blobs download.

In order to fix concurrent issue, need to create image record after
blobs download.

Fix: #3937

Signed-off-by: Wei Fu <fuweid89@gmail.com>
---
 client.go |  6 +++++-
 pull.go   | 30 ++++++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/client.go b/client.go
index f65b03827..f57d92031 100644
--- a/client.go
+++ b/client.go
@@ -390,7 +390,11 @@ func (c *Client) Fetch(ctx context.Context, ref string, opts ...RemoteOpt) (imag
 	}
 	defer done(ctx)
 
-	return c.fetch(ctx, fetchCtx, ref, 0)
+	img, err := c.fetch(ctx, fetchCtx, ref, 0)
+	if err != nil {
+		return images.Image{}, err
+	}
+	return c.createNewImage(ctx, img)
 }
 
 // Push uploads the provided content to a remote resource
diff --git a/pull.go b/pull.go
index 02d6f4e9b..c4cd919b5 100644
--- a/pull.go
+++ b/pull.go
@@ -27,6 +27,7 @@ import (
 	"github.com/containerd/containerd/remotes/docker/schema1"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
+	"golang.org/x/sync/errgroup"
 	"golang.org/x/sync/semaphore"
 )
 
@@ -62,15 +63,18 @@ func (c *Client) Pull(ctx context.Context, ref string, opts ...RemoteOpt) (_ Ima
 	defer done(ctx)
 
 	var unpacks int32
+	var unpackEg *errgroup.Group
+	var unpackWrapper func(f images.Handler) images.Handler
+
 	if pullCtx.Unpack {
 		// unpacker only supports schema 2 image, for schema 1 this is noop.
 		u, err := c.newUnpacker(ctx, pullCtx)
 		if err != nil {
 			return nil, errors.Wrap(err, "create unpacker")
 		}
-		unpackWrapper, eg := u.handlerWrapper(ctx, &unpacks)
+		unpackWrapper, unpackEg = u.handlerWrapper(ctx, &unpacks)
 		defer func() {
-			if err := eg.Wait(); err != nil {
+			if err := unpackEg.Wait(); err != nil {
 				if retErr == nil {
 					retErr = errors.Wrap(err, "unpack")
 				}
@@ -90,6 +94,22 @@ func (c *Client) Pull(ctx context.Context, ref string, opts ...RemoteOpt) (_ Ima
 		return nil, err
 	}
 
+	// NOTE(fuweid): unpacker defers blobs download. before create image
+	// record in ImageService, should wait for unpacking(including blobs
+	// download).
+	if pullCtx.Unpack {
+		if unpackEg != nil {
+			if err := unpackEg.Wait(); err != nil {
+				return nil, err
+			}
+		}
+	}
+
+	img, err = c.createNewImage(ctx, img)
+	if err != nil {
+		return nil, err
+	}
+
 	i := NewImageWithPlatform(c, img, pullCtx.PlatformMatcher)
 
 	if pullCtx.Unpack {
@@ -201,12 +221,14 @@ func (c *Client) fetch(ctx context.Context, rCtx *RemoteContext, ref string, lim
 		}
 	}
 
-	img := images.Image{
+	return images.Image{
 		Name:   name,
 		Target: desc,
 		Labels: rCtx.Labels,
-	}
+	}, nil
+}
 
+func (c *Client) createNewImage(ctx context.Context, img images.Image) (images.Image, error) {
 	is := c.ImageService()
 	for {
 		if created, err := is.Create(ctx, img); err != nil {