From de26af4fde9d8fb10b4f2f92446657981ffa6ee8 Mon Sep 17 00:00:00 2001
From: Anthony Emengo <aemengo@vmware.com>
Date: Mon, 26 Apr 2021 14:01:59 -0400
Subject: [PATCH] Modify fetcher

When Docker Daemon is available, defer to Docker Daemon when pulling remote images.
The motivation is that the Docker Daemon already handles more edge cases that we do.

Signed-off-by: Anthony Emengo <aemengo@vmware.com>
---
 internal/image/fetcher.go      | 64 +++++++++++++++++++---------------
 internal/image/fetcher_test.go | 15 +++++---
 2 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/internal/image/fetcher.go b/internal/image/fetcher.go
index c1c1e47604..b96870e527 100644
--- a/internal/image/fetcher.go
+++ b/internal/image/fetcher.go
@@ -4,12 +4,15 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"io"
+	"regexp"
 	"strings"
 
+	"github.com/buildpacks/imgutil/remote"
+
 	"github.com/buildpacks/imgutil"
 	"github.com/buildpacks/imgutil/local"
-	"github.com/buildpacks/imgutil/remote"
 	"github.com/buildpacks/lifecycle/auth"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/client"
@@ -38,50 +41,52 @@ func NewFetcher(logger logging.Logger, docker client.CommonAPIClient) *Fetcher {
 var ErrNotFound = errors.New("not found")
 
 func (f *Fetcher) Fetch(ctx context.Context, name string, daemon bool, pullPolicy config.PullPolicy) (imgutil.Image, error) {
-	if daemon {
-		if pullPolicy == config.PullNever {
-			return f.fetchDaemonImage(name)
-		} else if pullPolicy == config.PullIfNotPresent {
-			img, err := f.fetchDaemonImage(name)
-			if err == nil || !errors.Is(err, ErrNotFound) {
-				return img, err
-			}
+	if !daemon {
+		return f.fetchRemoteImage(name)
+	}
+
+	switch pullPolicy {
+	case config.PullNever:
+		img, err := f.fetchDaemonImage(name)
+		return img, err
+	case config.PullIfNotPresent:
+		img, err := f.fetchDaemonImage(name)
+		if err == nil || !errors.Is(err, ErrNotFound) {
+			return img, err
 		}
 	}
 
-	image, err := remote.NewImage(name, authn.DefaultKeychain, remote.FromBaseImage(name))
-	if err != nil {
+	f.logger.Debugf("Pulling image %s", style.Symbol(name))
+	if err := f.pullImage(ctx, name); err != nil {
 		return nil, err
 	}
 
-	remoteFound := image.Found()
+	return f.fetchDaemonImage(name)
+}
 
-	if daemon {
-		if remoteFound {
-			f.logger.Debugf("Pulling image %s", style.Symbol(name))
-			if err := f.pullImage(ctx, name); err != nil {
-				return nil, err
-			}
-		}
-		return f.fetchDaemonImage(name)
+func (f *Fetcher) fetchDaemonImage(name string) (imgutil.Image, error) {
+	image, err := local.NewImage(name, f.docker, local.FromBaseImage(name))
+	if err != nil {
+		return nil, err
 	}
 
-	if !remoteFound {
-		return nil, errors.Wrapf(ErrNotFound, "image %s does not exist in registry", style.Symbol(name))
+	if !image.Found() {
+		return nil, errors.Wrapf(ErrNotFound, "image %s does not exist on the daemon", style.Symbol(name))
 	}
 
 	return image, nil
 }
 
-func (f *Fetcher) fetchDaemonImage(name string) (imgutil.Image, error) {
-	image, err := local.NewImage(name, f.docker, local.FromBaseImage(name))
+func (f *Fetcher) fetchRemoteImage(name string) (imgutil.Image, error) {
+	image, err := remote.NewImage(name, authn.DefaultKeychain, remote.FromBaseImage(name))
 	if err != nil {
 		return nil, err
 	}
 
 	if !image.Found() {
-		return nil, errors.Wrapf(ErrNotFound, "image %s does not exist on the daemon", style.Symbol(name))
+		return nil, errors.Wrapf(ErrNotFound, "image %s does not exist in registry", style.Symbol(name))
 	}
+
 	return image, nil
 }
 
@@ -90,10 +95,13 @@ func (f *Fetcher) pullImage(ctx context.Context, imageID string) error {
 	if err != nil {
 		return err
 	}
-	rc, err := f.docker.ImagePull(ctx, imageID, types.ImagePullOptions{
-		RegistryAuth: regAuth,
-	})
+
+	rc, err := f.docker.ImagePull(ctx, imageID, types.ImagePullOptions{RegistryAuth: regAuth})
 	if err != nil {
+		if ok, _ := regexp.MatchString(fmt.Sprintf(`manifest for %s.* not found: manifest unknown: manifest unknown`, imageID), err.Error()); ok {
+			return errors.Wrapf(ErrNotFound, "image %s does not exist on the daemon", style.Symbol(imageID))
+		}
+
 		return err
 	}
 
diff --git a/internal/image/fetcher_test.go b/internal/image/fetcher_test.go
index c6f5dbf4de..07d0961dce 100644
--- a/internal/image/fetcher_test.go
+++ b/internal/image/fetcher_test.go
@@ -48,16 +48,23 @@ func TestFetcher(t *testing.T) {
 
 func testFetcher(t *testing.T, when spec.G, it spec.S) {
 	var (
-		fetcher  *image.Fetcher
-		repoName string
-		repo     string
-		outBuf   bytes.Buffer
+		fetcher           *image.Fetcher
+		repoName          string
+		repo              string
+		outBuf            bytes.Buffer
+		runnableBaseImage string
 	)
 
 	it.Before(func() {
 		repo = "some-org/" + h.RandString(10)
 		repoName = registryConfig.RepoName(repo)
 		fetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker)
+
+		daemonInfo, err := docker.Info(context.TODO())
+		h.AssertNil(t, err)
+
+		runnableBaseImage = h.RunnableBaseImage(daemonInfo.OSType)
+		h.PullIfMissing(t, docker, runnableBaseImage)
 	})
 
 	when("#Fetch", func() {