From a6e458caf1aca5deda542269714cde53becf1544 Mon Sep 17 00:00:00 2001
From: Cole Wippern <cgwippern@google.com>
Date: Fri, 22 Nov 2019 12:11:48 -0800
Subject: [PATCH] Update error handling and logging for cache

Previously we returned a low level file system error when checking for
a cached image. By adding a more human friendly log message and explicit
error handling we improve upon the user experience.
---
 pkg/cache/cache.go     |  5 +++--
 pkg/util/image_util.go | 31 ++++++++++++++++++-------------
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go
index 0953a28f65..d8c7898ac3 100644
--- a/pkg/cache/cache.go
+++ b/pkg/cache/cache.go
@@ -28,7 +28,7 @@ import (
 	"github.com/GoogleContainerTools/kaniko/pkg/config"
 	"github.com/GoogleContainerTools/kaniko/pkg/creds"
 	"github.com/google/go-containerregistry/pkg/name"
-	"github.com/google/go-containerregistry/pkg/v1"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/google/go-containerregistry/pkg/v1/tarball"
 	"github.com/pkg/errors"
@@ -124,7 +124,8 @@ func LocalSource(opts *config.CacheOptions, cacheKey string) (v1.Image, error) {
 
 	fi, err := os.Stat(path)
 	if err != nil {
-		return nil, errors.Wrap(err, "getting file info")
+		logrus.Debugf("No file found for cache key %v %v", cacheKey, err)
+		return nil, nil
 	}
 
 	// A stale cache is a bad cache
diff --git a/pkg/util/image_util.go b/pkg/util/image_util.go
index 01dc12faea..e6f653cee5 100644
--- a/pkg/util/image_util.go
+++ b/pkg/util/image_util.go
@@ -74,15 +74,13 @@ func RetrieveSourceImage(stage config.KanikoStage, opts *config.KanikoOptions) (
 	// If so, look in the local cache before trying the remote registry
 	if opts.CacheDir != "" {
 		cachedImage, err := cachedImage(opts, currentBaseName)
-		if cachedImage != nil {
-			return cachedImage, nil
-		}
-
 		if err != nil {
-			logrus.Infof("Error while retrieving image from cache: %v", err)
+			logrus.Errorf("Error while retrieving image from cache: %v %v", currentBaseName, err)
+		} else if cachedImage != nil {
+			return cachedImage, nil
 		}
 	}
-
+	logrus.Infof("Image %v not found in cache", currentBaseName)
 	// Otherwise, initialize image as usual
 	return RetrieveRemoteImage(currentBaseName, opts)
 }
@@ -93,13 +91,23 @@ func tarballImage(index int) (v1.Image, error) {
 	return tarball.ImageFromPath(tarPath, nil)
 }
 
+// Retrieves the manifest for the specified image from the specified registry
 func remoteImage(image string, opts *config.KanikoOptions) (v1.Image, error) {
-	logrus.Infof("Downloading base image %s", image)
+	logrus.Infof("Retrieving image manifest %s", image)
 	ref, err := name.ParseReference(image, name.WeakValidation)
 	if err != nil {
 		return nil, err
 	}
 
+	rOpts, err := prepareRemoteRequest(ref, opts)
+	if err != nil {
+		return nil, err
+	}
+
+	return remote.Image(ref, rOpts...)
+}
+
+func prepareRemoteRequest(ref name.Reference, opts *config.KanikoOptions) ([]remote.Option, error) {
 	registryName := ref.Context().RegistryStr()
 	if opts.InsecurePull || opts.InsecureRegistries.Contains(registryName) {
 		newReg, err := name.NewRegistry(registryName, name.WeakValidation, name.Insecure)
@@ -122,8 +130,7 @@ func remoteImage(image string, opts *config.KanikoOptions) (v1.Image, error) {
 			InsecureSkipVerify: true,
 		}
 	}
-
-	return remote.Image(ref, remote.WithTransport(tr), remote.WithAuthFromKeychain(creds.GetKeychain()))
+	return []remote.Option{remote.WithTransport(tr), remote.WithAuthFromKeychain(creds.GetKeychain())}, nil
 }
 
 func cachedImage(opts *config.KanikoOptions, image string) (v1.Image, error) {
@@ -136,18 +143,16 @@ func cachedImage(opts *config.KanikoOptions, image string) (v1.Image, error) {
 	if d, ok := ref.(name.Digest); ok {
 		cacheKey = d.DigestStr()
 	} else {
-		img, err := remoteImage(image, opts)
+		image, err := remoteImage(image, opts)
 		if err != nil {
 			return nil, err
 		}
 
-		d, err := img.Digest()
+		d, err := image.Digest()
 		if err != nil {
 			return nil, err
 		}
-
 		cacheKey = d.String()
 	}
-
 	return cache.LocalSource(&opts.CacheOptions, cacheKey)
 }