diff --git a/lib/builder/build_node.go b/lib/builder/build_node.go index 706066a4..54b00c56 100644 --- a/lib/builder/build_node.go +++ b/lib/builder/build_node.go @@ -105,13 +105,13 @@ func (n *buildNode) doCommit(cacheMgr cache.Manager, opts *buildOptions) error { return fmt.Errorf("commit: %s", err) } - // If the number of digestPairs is 0 or greater than 1 then we cannot push + // If the number of digestPairs is greater than 1 then we cannot push // the resulting layer mappings to the distributed cache. - if len(n.digestPairs) != 1 { + if len(n.digestPairs) > 1 { return nil } - if err := n.pushCacheLayers(cacheMgr); err != nil { + if err := n.pushCacheLayer(cacheMgr); err != nil { return fmt.Errorf("push cache: %s", err) } return nil @@ -147,20 +147,28 @@ func (n *buildNode) applyLayer(digestPair *image.DigestPair, modifyfs bool) erro } // pushCacheLayers pushs cached layers for this node's digest pair(s). -func (n *buildNode) pushCacheLayers(cacheMgr cache.Manager) error { - digestPair := n.digestPairs[0] - log.Infof("* Committed gzipped layer %s (%d bytes)", - digestPair.GzipDescriptor.Digest, digestPair.GzipDescriptor.Size) +func (n *buildNode) pushCacheLayer(cacheMgr cache.Manager) error { + var digestPair *image.DigestPair + if len(n.digestPairs) != 0 { + digestPair = n.digestPairs[0] + } + + if digestPair != nil { + log.Infof("* Committed gzipped layer %s (%d bytes)", + digestPair.GzipDescriptor.Digest, digestPair.GzipDescriptor.Size) + } log.Infof("* Pushing with cache ID %s", n.CacheID()) return cacheMgr.PushCache(n.CacheID(), digestPair) } -// pullCacheLayers pulls cached layers for this node's digest pair(s). -func (n *buildNode) pullCacheLayers(cacheMgr cache.Manager) bool { +// pullCacheLayer pulls cached layers for this node's digest pair(s). +func (n *buildNode) pullCacheLayer(cacheMgr cache.Manager) bool { digestPair, err := cacheMgr.PullCache(n.CacheID()) if err != nil { log.Errorf("Failed to fetch intermediate layer with cache ID %s: %s", n.CacheID(), err) return false + } else if digestPair == nil { + return true } n.digestPairs = []*image.DigestPair{digestPair} return true diff --git a/lib/builder/build_stage.go b/lib/builder/build_stage.go index 0269add8..9b3c63d0 100644 --- a/lib/builder/build_stage.go +++ b/lib/builder/build_stage.go @@ -245,8 +245,8 @@ func (stage *buildStage) pullCacheLayers(cacheMgr cache.Manager) { // it gets executed. for _, node := range stage.nodes[1:] { // Stop once the cache chain is broken. - if node.HasCommit() { - if !node.pullCacheLayers(cacheMgr) { + if node.HasCommit() || stage.forceCommit { + if !node.pullCacheLayer(cacheMgr) { return } } diff --git a/lib/cache/cache_manager.go b/lib/cache/cache_manager.go index 7b86401a..d8c83563 100644 --- a/lib/cache/cache_manager.go +++ b/lib/cache/cache_manager.go @@ -29,6 +29,7 @@ import ( ) const _cachePrefix = "makisu_builder_cache_" +const _cacheEmptyEntry = "MAKISU_CACHE_EMPTY" // Manager is the interface through which we interact with the cacheID -> image layer mapping. type Manager interface { @@ -114,6 +115,10 @@ func (manager *registryCacheManager) PullCache(cacheID string) (*image.DigestPai } log.Infof("Found mapping in cacheID KVStore: %s => %s", cacheID, entry) + if entry == _cacheEmptyEntry { + return nil, nil + } + tarDigest, gzipDigest, err := parseEntry(entry) if err != nil { return nil, errors.Wrapf(ErrorLayerNotFound, "parse entry %s", entry) @@ -149,12 +154,14 @@ func (manager *registryCacheManager) PushCache(cacheID string, digestPair *image manager.Lock() defer manager.Unlock() - if err := manager.registryClient.PushLayer(digestPair.GzipDescriptor.Digest); err != nil { - manager.pushErrors.Add(fmt.Errorf("push layer %s: %s", digestPair.GzipDescriptor.Digest, err)) - return + if digestPair != nil { + if err := manager.registryClient.PushLayer(digestPair.GzipDescriptor.Digest); err != nil { + manager.pushErrors.Add(fmt.Errorf("push layer %s: %s", digestPair.GzipDescriptor.Digest, err)) + return + } } - entry := createEntry(digestPair.TarDigest, digestPair.GzipDescriptor.Digest) + entry := createEntry(digestPair) if err := manager.cacheIDStore.Put(_cachePrefix+cacheID, entry); err != nil { manager.pushErrors.Add(fmt.Errorf("store tag mapping (%s,%s): %s", cacheID, entry, err)) return @@ -189,6 +196,9 @@ func parseEntry(entry string) (image.Digest, image.Digest, error) { return image.Digest("sha256:" + split[0]), image.Digest("sha256:" + split[1]), nil } -func createEntry(tarDigest, gzipDigest image.Digest) string { - return fmt.Sprintf("%s,%s", tarDigest.Hex(), gzipDigest.Hex()) +func createEntry(pair *image.DigestPair) string { + if pair == nil { + return _cacheEmptyEntry + } + return fmt.Sprintf("%s,%s", pair.TarDigest.Hex(), pair.GzipDescriptor.Digest.Hex()) } diff --git a/test/python/test_build.py b/test/python/test_build.py index 4ce364e2..54f7c51e 100644 --- a/test/python/test_build.py +++ b/test/python/test_build.py @@ -108,6 +108,24 @@ def test_build_go_from_scratch(registry1, storage_dir): assert code == 0, err +def test_build_commit_empty_pair(registry1, storage_dir, cache_dir, tmpdir): + utils.registry_ensure_image('debian:8', registry1.addr) + new_image1 = new_image_name() + new_image2 = new_image_name() + context_dir = os.path.join(os.getcwd(), 'testdata/build-context/commit-empty-pair') + test_file = tmpdir.mkdir("d1").join("f1") + test_file.write("hello") + + # First build, mount in test file. + additional_volumes = {test_file: '/mnt/f1'} + utils.makisu_build_image(new_image1, registry1.addr, context_dir, storage_dir, cache_dir, additional_volumes) + + # Second build, without test file. It would fail if distributed cache doesn't work. + utils.makisu_build_image(new_image2, registry1.addr, context_dir, storage_dir, cache_dir) + code, err = utils.docker_run_image(registry1.addr, new_image2) + assert code == 0, err + + def test_build_with_cache(registry1, storage_dir, cache_dir): utils.registry_ensure_image('debian:8', registry1.addr) new_image1 = new_image_name() diff --git a/testdata/build-context/commit-empty-pair/Dockerfile b/testdata/build-context/commit-empty-pair/Dockerfile new file mode 100644 index 00000000..5df0609f --- /dev/null +++ b/testdata/build-context/commit-empty-pair/Dockerfile @@ -0,0 +1,5 @@ +FROM busybox +RUN mkdir /test #!COMMIT +WORKDIR /test #!COMMIT +RUN cp /mnt/f1 f1 #!COMMIT +RUN ls /test/f1