Try to import docker images before falling back to building

docs/content/en/schemas/v2beta8.json

@@ -1872,6 +1872,12 @@
           "description": "should images be pushed to a registry. If not specified, images are pushed only if the current Kubernetes context connects to a remote cluster.",
           "x-intellij-html-description": "should images be pushed to a registry. If not specified, images are pushed only if the current Kubernetes context connects to a remote cluster."
         },
+        "tryImportMissing": {
+          "type": "boolean",
+          "description": "whether to attempt to import artifacts from Docker (either a local or remote registry) if not in the cache.",
+          "x-intellij-html-description": "whether to attempt to import artifacts from Docker (either a local or remote registry) if not in the cache.",
+          "default": "false"
+        },
         "useBuildkit": {
           "type": "boolean",
           "description": "use BuildKit to build Docker images.",
@@ -1887,6 +1893,7 @@
       },
       "preferredOrder": [
         "push",
+        "tryImportMissing",
         "useDockerCLI",
         "useBuildkit",
         "concurrency"

pkg/skaffold/build/cache/cache.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
+	"sync"
 
 	homedir "github.com/mitchellh/go-homedir"
 	"github.com/sirupsen/logrus"
@@ -45,10 +46,12 @@ type ArtifactCache map[string]ImageDetails
 // cache holds any data necessary for accessing the cache
 type cache struct {
 	artifactCache      ArtifactCache
+	cacheMutex         sync.RWMutex
 	client             docker.LocalDaemon
 	insecureRegistries map[string]bool
 	cacheFile          string
 	imagesAreLocal     bool
+	tryImportMissing   bool
 	hashForArtifact    func(ctx context.Context, a *latest.Artifact) (string, error)
 }
 
@@ -64,7 +67,7 @@ type Config interface {
 }
 
 // NewCache returns the current state of the cache
-func NewCache(cfg Config, imagesAreLocal bool, dependencies DependencyLister) (Cache, error) {
+func NewCache(cfg Config, imagesAreLocal bool, tryImportMissing bool, dependencies DependencyLister) (Cache, error) {
 	if !cfg.CacheArtifacts() {
 		return &noCache{}, nil
 	}
@@ -92,6 +95,7 @@ func NewCache(cfg Config, imagesAreLocal bool, dependencies DependencyLister) (C
 		insecureRegistries: cfg.GetInsecureRegistries(),
 		cacheFile:          cacheFile,
 		imagesAreLocal:     imagesAreLocal,
+		tryImportMissing:   tryImportMissing,
 		hashForArtifact: func(ctx context.Context, a *latest.Artifact) (string, error) {
 			return getHashForArtifact(ctx, dependencies, a, cfg.Mode())
 		},

pkg/skaffold/build/cache/details.go

@@ -111,8 +111,10 @@ func (d needsPushing) Push(ctx context.Context, out io.Writer, c *cache) error {
 	}
 
 	// Update cache
+	c.cacheMutex.Lock()
 	e := c.artifactCache[d.hash]
 	e.Digest = digest
 	c.artifactCache[d.hash] = e
+	c.cacheMutex.Unlock()
 	return nil
 }

pkg/skaffold/build/cache/lookup.go

@@ -19,8 +19,11 @@ package cache
 import (
 	"context"
 	"fmt"
+	"io/ioutil"
 	"sync"
 
+	"github.com/sirupsen/logrus"
+
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
@@ -50,9 +53,14 @@ func (c *cache) lookup(ctx context.Context, a *latest.Artifact, tag string) cach
 		return failed{err: fmt.Errorf("getting hash for artifact %q: %s", a.ImageName, err)}
 	}
 
+	c.cacheMutex.RLock()
 	entry, cacheHit := c.artifactCache[hash]
+	c.cacheMutex.RUnlock()
 	if !cacheHit {
-		return needsBuilding{hash: hash}
+		if entry, err = c.tryImport(ctx, a, tag, hash); err != nil {
+			logrus.Debugf("Could not import artifact from Docker, building instead (%s)", err)
+			return needsBuilding{hash: hash}
+		}
 	}
 
 	if c.imagesAreLocal {
@@ -108,3 +116,40 @@ func (c *cache) lookupRemote(ctx context.Context, hash, tag string, entry ImageD
 
 	return needsBuilding{hash: hash}
 }
+
+func (c *cache) tryImport(ctx context.Context, a *latest.Artifact, tag string, hash string) (ImageDetails, error) {
+	if !c.tryImportMissing {
+		return ImageDetails{}, fmt.Errorf("import of missing images disabled")
+	}
+
+	entry := ImageDetails{}
+
+	if !c.client.ImageExists(ctx, tag) {
+		logrus.Debugf("Importing artifact %s from docker registry", tag)
+		err := c.client.Pull(ctx, ioutil.Discard, tag)
+		if err != nil {
+			return entry, err
+		}
+	} else {
+		logrus.Debugf("Importing artifact %s from local docker", tag)
+	}
+
+	imageID, err := c.client.ImageID(ctx, a.ImageName)
+	if err != nil {
+		return entry, err
+	}
+
+	if imageID != "" {
+		entry.ID = imageID
+	}
+
+	if digest, err := docker.RemoteDigest(tag, c.insecureRegistries); err == nil {
+		logrus.Debugf("Added digest for %s to cache entry", tag)
+		entry.Digest = digest
+	}
+
+	c.cacheMutex.Lock()
+	c.artifactCache[hash] = entry
+	c.cacheMutex.Unlock()
+	return entry, nil
+}

pkg/skaffold/build/cache/lookup_test.go

@@ -40,6 +40,8 @@ func TestLookupLocal(t *testing.T) {
 		{
 			description: "miss",
 			hasher:      mockHasher("thehash"),
+			api:         &testutil.FakeAPIClient{},
+			cache:       map[string]ImageDetails{},
 			expected:    needsBuilding{hash: "thehash"},
 		},
 		{
@@ -134,6 +136,8 @@ func TestLookupRemote(t *testing.T) {
 		{
 			description: "miss",
 			hasher:      mockHasher("hash"),
+			api:         &testutil.FakeAPIClient{ErrImagePull: true},
+			cache:       map[string]ImageDetails{},
 			expected:    needsBuilding{hash: "hash"},
 		},
 		{

pkg/skaffold/build/cache/retrieve.go

@@ -89,7 +89,9 @@ func (c *cache) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, ar
 		}
 
 		// Image is already built
+		c.cacheMutex.RLock()
 		entry := c.artifactCache[result.Hash()]
+		c.cacheMutex.RUnlock()
 		tag := tags[artifact.ImageName]
 
 		var uniqueTag string
@@ -166,7 +168,9 @@ func (c *cache) addArtifacts(ctx context.Context, bRes []build.Artifact, hashByN
 			entry.ID = imageID
 		}
 
+		c.cacheMutex.Lock()
 		c.artifactCache[hashByName[a.ImageName]] = entry
+		c.cacheMutex.Unlock()
 	}
 
 	return nil

pkg/skaffold/build/cache/retrieve_test.go

@@ -129,7 +129,7 @@ func TestCacheBuildLocal(t *testing.T) {
 		cfg := &mockConfig{
 			cacheFile: tmpDir.Path("cache"),
 		}
-		artifactCache, err := NewCache(cfg, true, deps)
+		artifactCache, err := NewCache(cfg, true, false, deps)
 		t.CheckNoError(err)
 
 		// First build: Need to build both artifacts
@@ -224,7 +224,7 @@ func TestCacheBuildRemote(t *testing.T) {
 		cfg := &mockConfig{
 			cacheFile: tmpDir.Path("cache"),
 		}
-		artifactCache, err := NewCache(cfg, false, deps)
+		artifactCache, err := NewCache(cfg, false, false, deps)
 		t.CheckNoError(err)
 
 		// First build: Need to build both artifacts
@@ -261,6 +261,69 @@ func TestCacheBuildRemote(t *testing.T) {
 	})
 }
 
+func TestCacheFindMissing(t *testing.T) {
+	testutil.Run(t, "", func(t *testutil.T) {
+		tmpDir := t.NewTempDir().
+			Write("dep1", "content1").
+			Write("dep2", "content2").
+			Write("dep3", "content3").
+			Chdir()
+
+		tags := map[string]string{
+			"artifact1": "artifact1:tag1",
+			"artifact2": "artifact2:tag2",
+		}
+		artifacts := []*latest.Artifact{
+			{ImageName: "artifact1", ArtifactType: latest.ArtifactType{DockerArtifact: &latest.DockerArtifact{}}},
+			{ImageName: "artifact2", ArtifactType: latest.ArtifactType{DockerArtifact: &latest.DockerArtifact{}}},
+		}
+		deps := depLister(map[string][]string{
+			"artifact1": {"dep1", "dep2"},
+			"artifact2": {"dep3"},
+		})
+
+		// Mock Docker
+		dockerDaemon := fakeLocalDaemon(&testutil.FakeAPIClient{})
+		t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) {
+			return dockerDaemon, nil
+		})
+		t.Override(&docker.DefaultAuthHelper, stubAuth{})
+		t.Override(&docker.RemoteDigest, func(ref string, _ map[string]bool) (string, error) {
+			switch ref {
+			case "artifact1:tag1":
+				return "sha256:51ae7fa00c92525c319404a3a6d400e52ff9372c5a39cb415e0486fe425f3165", nil
+			case "artifact2:tag2":
+				return "sha256:35bdf2619f59e6f2372a92cb5486f4a0bf9b86e0e89ee0672864db6ed9c51539", nil
+			default:
+				return "", errors.New("unknown remote tag")
+			}
+		})
+
+		// Mock args builder
+		t.Override(&docker.EvalBuildArgs, func(mode config.RunMode, workspace string, a *latest.DockerArtifact) (map[string]*string, error) {
+			return a.BuildArgs, nil
+		})
+
+		// Create cache
+		cfg := &mockConfig{
+			cacheFile: tmpDir.Path("cache"),
+		}
+		artifactCache, err := NewCache(cfg, false, true, deps)
+		t.CheckNoError(err)
+
+		// Because the artifacts are in the docker registry, we expect them to be imported correctly.
+		builder := &mockBuilder{dockerDaemon: dockerDaemon, push: false}
+		bRes, err := artifactCache.Build(context.Background(), ioutil.Discard, tags, artifacts, builder.BuildAndTest)
+
+		t.CheckNoError(err)
+		t.CheckDeepEqual(0, len(builder.built))
+		t.CheckDeepEqual(2, len(bRes))
+		// Artifacts should always be returned in their original order
+		t.CheckDeepEqual("artifact1", bRes[0].ImageName)
+		t.CheckDeepEqual("artifact2", bRes[1].ImageName)
+	})
+}
+
 type mockConfig struct {
 	runcontext.RunContext // Embedded to provide the default values.
 	cacheFile             string

pkg/skaffold/build/local/types.go

@@ -36,6 +36,7 @@ type Builder struct {
 	localDocker        docker.LocalDaemon
 	localCluster       bool
 	pushImages         bool
+	tryImportMissing   bool
 	prune              bool
 	pruneChildren      bool
 	skipTests          bool
@@ -88,12 +89,15 @@ func NewBuilder(cfg Config) (*Builder, error) {
 		pushImages = *cfg.Pipeline().Build.LocalBuild.Push
 	}
 
+	tryImportMissing := cfg.Pipeline().Build.LocalBuild.TryImportMissing
+
 	return &Builder{
 		cfg:                *cfg.Pipeline().Build.LocalBuild,
 		kubeContext:        cfg.GetKubeContext(),
 		localDocker:        localDocker,
 		localCluster:       localCluster,
 		pushImages:         pushImages,
+		tryImportMissing:   tryImportMissing,
 		skipTests:          cfg.SkipTests(),
 		mode:               cfg.Mode(),
 		prune:              cfg.Prune(),
@@ -107,6 +111,10 @@ func (b *Builder) PushImages() bool {
 	return b.pushImages
 }
 
+func (b *Builder) TryImportMissing() bool {
+	return b.tryImportMissing
+}
+
 // Prune uses the docker API client to remove all images built with Skaffold
 func (b *Builder) Prune(ctx context.Context, out io.Writer) error {
 	return b.localDocker.Prune(ctx, out, b.builtImages, b.pruneChildren)

pkg/skaffold/runner/new.go

@@ -55,6 +55,11 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) {
 		return nil, fmt.Errorf("creating builder: %w", err)
 	}
 
+	tryImportMissing := false
+	if localBuilder, ok := builder.(*local.Builder); ok {
+		tryImportMissing = localBuilder.TryImportMissing()
+	}
+
 	labeller := deploy.NewLabeller(runCtx.AddSkaffoldLabels(), runCtx.CustomLabels())
 	tester := getTester(runCtx, imagesAreLocal)
 	syncer := getSyncer(runCtx)
@@ -74,7 +79,7 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) {
 		return append(buildDependencies, testDependencies...), nil
 	}
 
-	artifactCache, err := cache.NewCache(runCtx, imagesAreLocal, depLister)
+	artifactCache, err := cache.NewCache(runCtx, imagesAreLocal, tryImportMissing, depLister)
 	if err != nil {
 		return nil, fmt.Errorf("initializing cache: %w", err)
 	}

pkg/skaffold/schema/latest/config.go

@@ -217,6 +217,10 @@ type LocalBuild struct {
 	// connects to a remote cluster.
 	Push *bool `yaml:"push,omitempty"`
 
+	// TryImportMissing whether to attempt to import artifacts from
+	// Docker (either a local or remote registry) if not in the cache.
+	TryImportMissing bool `yaml:"tryImportMissing,omitempty"`
+
 	// UseDockerCLI use `docker` command-line interface instead of Docker Engine APIs.
 	UseDockerCLI bool `yaml:"useDockerCLI,omitempty"`