From df6be8824071e9fe0f89faa50a610daa23e46573 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Wed, 17 Jul 2024 10:39:20 -0400 Subject: [PATCH] Recover corrupted cache (#1381) * add failing test to restorer Signed-off-by: Joey Brown * restorer and exporter working as expected Signed-off-by: Joey Brown * lint Signed-off-by: Joey Brown * Update phase/restorer.go Co-authored-by: Natalie Arellano Signed-off-by: Joey Brown * Update phase/cache.go Co-authored-by: Natalie Arellano Signed-off-by: Joey Brown * Update cache/image_cache.go Co-authored-by: Natalie Arellano Signed-off-by: Joey Brown * Update cache/volume_cache.go Co-authored-by: Natalie Arellano Signed-off-by: Joey Brown * Update cache/volume_cache.go Co-authored-by: Natalie Arellano Signed-off-by: Joey Brown * Update cache/volume_cache.go Co-authored-by: Natalie Arellano Signed-off-by: Joey Brown * Update cache/volume_cache.go Co-authored-by: Natalie Arellano Signed-off-by: Joey Brown * Update cache/volume_cache.go Co-authored-by: Natalie Arellano Signed-off-by: Joey Brown * update based on feedback Signed-off-by: Joey Brown * fix log * temp fix * this does not work as is. I think we need to modify img utils. Image utils should fail with a Layer Not found in both ReuseLayer & GetLayer. For GetLayer, when there is a missing blob, it's return an unexpected EOF error. For ReuseLayer, when there is a missing blob, it's not returning an error but it should. * add eof check * add not exist check * reuse layer test * fix test regression --------- Signed-off-by: Joey Brown Co-authored-by: Joey Brown --- cache/caching_image_test.go | 4 +- cache/common.go | 22 ++++ cache/image_cache.go | 33 +++++- cache/image_cache_test.go | 4 +- cache/image_deleter.go | 7 +- cache/volume_cache.go | 33 +++++- cache/volume_cache_test.go | 40 ++++++-- cmd/lifecycle/exporter.go | 8 +- cmd/lifecycle/main.go | 8 +- internal/layer/sbom_restorer_test.go | 2 +- phase/analyzer.go | 2 +- phase/analyzer_test.go | 5 +- phase/cache.go | 10 +- phase/cache_test.go | 17 ++-- phase/connected_factory.go | 6 +- phase/restorer.go | 13 ++- phase/restorer_test.go | 140 ++++++++++++++++---------- phase/testmock/cache/image_deleter.go | 12 +++ 18 files changed, 269 insertions(+), 97 deletions(-) diff --git a/cache/caching_image_test.go b/cache/caching_image_test.go index 345acc2cc..31ebf3748 100644 --- a/cache/caching_image_test.go +++ b/cache/caching_image_test.go @@ -13,6 +13,8 @@ import ( "github.com/sclevine/spec" "github.com/sclevine/spec/report" + "github.com/buildpacks/lifecycle/cmd" + "github.com/buildpacks/lifecycle/cache" h "github.com/buildpacks/lifecycle/testhelpers" ) @@ -37,7 +39,7 @@ func testCachingImage(t *testing.T, when spec.G, it spec.S) { fakeImage = fakes.NewImage("some-image", "", nil) tmpDir, err = os.MkdirTemp("", "") h.AssertNil(t, err) - volumeCache, err = cache.NewVolumeCache(tmpDir) + volumeCache, err = cache.NewVolumeCache(tmpDir, cmd.DefaultLogger) h.AssertNil(t, err) subject = cache.NewCachingImage(fakeImage, volumeCache) layerPath, layerSHA, layerData = h.RandomLayer(t, tmpDir) diff --git a/cache/common.go b/cache/common.go index 8b06e362f..c37d2f20a 100644 --- a/cache/common.go +++ b/cache/common.go @@ -5,3 +5,25 @@ import ( ) var errCacheCommitted = errors.New("cache cannot be modified after commit") + +// ReadErr is an error type for filesystem read errors. +type ReadErr struct { + msg string +} + +// NewReadErr creates a new ReadErr. +func NewReadErr(msg string) ReadErr { + return ReadErr{msg: msg} +} + +// Error returns the error message. +func (e ReadErr) Error() string { + return e.msg +} + +// IsReadErr checks if an error is a ReadErr. +func IsReadErr(err error) (bool, *ReadErr) { + var e ReadErr + isReadErr := errors.As(err, &e) + return isReadErr, &e +} diff --git a/cache/image_cache.go b/cache/image_cache.go index efdbdf2e9..a029c87dc 100644 --- a/cache/image_cache.go +++ b/cache/image_cache.go @@ -99,15 +99,44 @@ func (c *ImageCache) AddLayerFile(tarPath string, diffID string) error { return c.newImage.AddLayerWithDiffID(tarPath, diffID) } +// isLayerNotFound checks if the error is a layer not found error +// +// FIXME: we should not have to rely on trapping ErrUnexpectedEOF. +// If a blob is not present in the registry, we should get imgutil.ErrLayerNotFound, +// but we do not and instead get io.ErrUnexpectedEOF +func isLayerNotFound(err error) bool { + var e imgutil.ErrLayerNotFound + return errors.As(err, &e) || errors.Is(err, io.ErrUnexpectedEOF) +} + func (c *ImageCache) ReuseLayer(diffID string) error { if c.committed { return errCacheCommitted } - return c.newImage.ReuseLayer(diffID) + err := c.newImage.ReuseLayer(diffID) + if err != nil { + // FIXME: this path is not currently executed. + // If a blob is not present in the registry, we should get imgutil.ErrLayerNotFound. + // We should then skip attempting to reuse the layer. + // However, we do not get imgutil.ErrLayerNotFound when the blob is not present. + if isLayerNotFound(err) { + return NewReadErr(fmt.Sprintf("failed to find cache layer with SHA '%s'", diffID)) + } + return fmt.Errorf("failed to reuse cache layer with SHA '%s'", diffID) + } + return nil } +// RetrieveLayer retrieves a layer from the cache func (c *ImageCache) RetrieveLayer(diffID string) (io.ReadCloser, error) { - return c.origImage.GetLayer(diffID) + closer, err := c.origImage.GetLayer(diffID) + if err != nil { + if isLayerNotFound(err) { + return nil, NewReadErr(fmt.Sprintf("failed to find cache layer with SHA '%s'", diffID)) + } + return nil, fmt.Errorf("failed to get cache layer with SHA '%s'", diffID) + } + return closer, nil } func (c *ImageCache) Commit() error { diff --git a/cache/image_cache_test.go b/cache/image_cache_test.go index d3fcb2d16..3a92c45e5 100644 --- a/cache/image_cache_test.go +++ b/cache/image_cache_test.go @@ -146,7 +146,7 @@ func testImageCache(t *testing.T, when spec.G, it spec.S) { when("layer does not exist", func() { it("returns an error", func() { _, err := subject.RetrieveLayer("some_nonexistent_sha") - h.AssertError(t, err, "failed to get layer with sha 'some_nonexistent_sha'") + h.AssertError(t, err, "failed to get cache layer with SHA 'some_nonexistent_sha'") }) }) }) @@ -236,7 +236,7 @@ func testImageCache(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.AddLayerFile(testLayerTarPath, testLayerSHA)) _, err := subject.RetrieveLayer(testLayerSHA) - h.AssertError(t, err, fmt.Sprintf("failed to get layer with sha '%s'", testLayerSHA)) + h.AssertError(t, err, fmt.Sprintf("failed to get cache layer with SHA '%s'", testLayerSHA)) }) }) }) diff --git a/cache/image_deleter.go b/cache/image_deleter.go index 8807715d1..f6fc03acd 100644 --- a/cache/image_deleter.go +++ b/cache/image_deleter.go @@ -12,6 +12,7 @@ import ( // ImageDeleter defines the methods available to delete and compare cached images type ImageDeleter interface { DeleteOrigImageIfDifferentFromNewImage(origImage, newImage imgutil.Image) + DeleteImage(image imgutil.Image) } // ImageDeleterImpl is a component to manage cache image deletion @@ -35,13 +36,13 @@ func (c *ImageDeleterImpl) DeleteOrigImageIfDifferentFromNewImage(origImage, new } if !same { - c.deleteImage(origImage) + c.DeleteImage(origImage) } } } -// deleteImage deletes an image -func (c *ImageDeleterImpl) deleteImage(image imgutil.Image) { +// DeleteImage deletes an image +func (c *ImageDeleterImpl) DeleteImage(image imgutil.Image) { if c.deletionEnabled { if err := image.Delete(); err != nil { c.logger.Warnf("Unable to delete cache image: %v", err.Error()) diff --git a/cache/volume_cache.go b/cache/volume_cache.go index 210e1afbf..3ec0003cc 100644 --- a/cache/volume_cache.go +++ b/cache/volume_cache.go @@ -2,6 +2,7 @@ package cache import ( "encoding/json" + "fmt" "io" "os" "path/filepath" @@ -10,6 +11,8 @@ import ( "github.com/pkg/errors" + "github.com/buildpacks/lifecycle/log" + "github.com/buildpacks/lifecycle/internal/fsutil" "github.com/buildpacks/lifecycle/platform" ) @@ -20,9 +23,11 @@ type VolumeCache struct { backupDir string stagingDir string committedDir string + logger log.Logger } -func NewVolumeCache(dir string) (*VolumeCache, error) { +// NewVolumeCache creates a new VolumeCache +func NewVolumeCache(dir string, logger log.Logger) (*VolumeCache, error) { if _, err := os.Stat(dir); err != nil { return nil, err } @@ -32,6 +37,7 @@ func NewVolumeCache(dir string) (*VolumeCache, error) { backupDir: filepath.Join(dir, "committed-backup"), stagingDir: filepath.Join(dir, "staging"), committedDir: filepath.Join(dir, "committed"), + logger: logger, } if err := c.setupStagingDir(); err != nil { @@ -133,7 +139,20 @@ func (c *VolumeCache) ReuseLayer(diffID string) error { if c.committed { return errCacheCommitted } - if err := os.Link(diffIDPath(c.committedDir, diffID), diffIDPath(c.stagingDir, diffID)); err != nil && !os.IsExist(err) { + committedPath := diffIDPath(c.committedDir, diffID) + stagingPath := diffIDPath(c.stagingDir, diffID) + + if _, err := os.Stat(committedPath); err != nil { + if os.IsNotExist(err) { + return NewReadErr(fmt.Sprintf("failed to find cache layer with SHA '%s'", diffID)) + } + if os.IsPermission(err) { + return NewReadErr(fmt.Sprintf("failed to read cache layer with SHA '%s' due to insufficient permissions", diffID)) + } + return fmt.Errorf("failed to re-use cache layer with SHA '%s': %w", diffID, err) + } + + if err := os.Link(committedPath, stagingPath); err != nil && !os.IsExist(err) { return errors.Wrapf(err, "reusing layer (%s)", diffID) } return nil @@ -146,7 +165,13 @@ func (c *VolumeCache) RetrieveLayer(diffID string) (io.ReadCloser, error) { } file, err := os.Open(path) if err != nil { - return nil, errors.Wrapf(err, "opening layer with SHA '%s'", diffID) + if os.IsPermission(err) { + return nil, NewReadErr(fmt.Sprintf("failed to read cache layer with SHA '%s' due to insufficient permissions", diffID)) + } + if os.IsNotExist(err) { + return nil, NewReadErr(fmt.Sprintf("failed to find cache layer with SHA '%s'", diffID)) + } + return nil, fmt.Errorf("failed to get cache layer with SHA '%s'", diffID) } return file, nil } @@ -165,7 +190,7 @@ func (c *VolumeCache) RetrieveLayerFile(diffID string) (string, error) { path := diffIDPath(c.committedDir, diffID) if _, err := os.Stat(path); err != nil { if os.IsNotExist(err) { - return "", errors.Wrapf(err, "layer with SHA '%s' not found", diffID) + return "", NewReadErr(fmt.Sprintf("failed to find cache layer with SHA '%s'", diffID)) } return "", errors.Wrapf(err, "retrieving layer with SHA '%s'", diffID) } diff --git a/cache/volume_cache_test.go b/cache/volume_cache_test.go index 3ab97ffe0..e7df2ef2a 100644 --- a/cache/volume_cache_test.go +++ b/cache/volume_cache_test.go @@ -10,6 +10,9 @@ import ( "github.com/sclevine/spec" "github.com/sclevine/spec/report" + "github.com/buildpacks/lifecycle/cmd" + "github.com/buildpacks/lifecycle/log" + "github.com/buildpacks/lifecycle/buildpack" "github.com/buildpacks/lifecycle/cache" "github.com/buildpacks/lifecycle/platform" @@ -28,6 +31,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { backupDir string stagingDir string committedDir string + testLogger log.Logger ) it.Before(func() { @@ -42,6 +46,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { backupDir = filepath.Join(volumeDir, "committed-backup") stagingDir = filepath.Join(volumeDir, "staging") committedDir = filepath.Join(volumeDir, "committed") + testLogger = cmd.DefaultLogger }) it.After(func() { @@ -50,7 +55,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { when("#NewVolumeCache", func() { it("returns an error when the volume path does not exist", func() { - _, err := cache.NewVolumeCache(filepath.Join(tmpDir, "does_not_exist")) + _, err := cache.NewVolumeCache(filepath.Join(tmpDir, "does_not_exist"), testLogger) if err == nil { t.Fatal("expected NewVolumeCache to fail because volume path does not exist") } @@ -66,7 +71,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { it("clears staging", func() { var err error - subject, err = cache.NewVolumeCache(volumeDir) + subject, err = cache.NewVolumeCache(volumeDir, testLogger) h.AssertNil(t, err) _, err = os.Stat(filepath.Join(stagingDir, "some-layer.tar")) @@ -80,7 +85,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { it("creates staging dir", func() { var err error - subject, err = cache.NewVolumeCache(volumeDir) + subject, err = cache.NewVolumeCache(volumeDir, testLogger) h.AssertNil(t, err) _, err = os.Stat(stagingDir) @@ -92,7 +97,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { it("creates committed dir", func() { var err error - subject, err = cache.NewVolumeCache(volumeDir) + subject, err = cache.NewVolumeCache(volumeDir, testLogger) h.AssertNil(t, err) _, err = os.Stat(committedDir) @@ -109,7 +114,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { it("clears the backup dir", func() { var err error - subject, err = cache.NewVolumeCache(volumeDir) + subject, err = cache.NewVolumeCache(volumeDir, testLogger) h.AssertNil(t, err) _, err = os.Stat(filepath.Join(backupDir, "some-layer.tar")) @@ -124,7 +129,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { it.Before(func() { var err error - subject, err = cache.NewVolumeCache(volumeDir) + subject, err = cache.NewVolumeCache(volumeDir, testLogger) h.AssertNil(t, err) }) @@ -206,7 +211,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { when("layer does not exist", func() { it("returns an error", func() { _, err := subject.RetrieveLayer("some_nonexistent_sha") - h.AssertError(t, err, "layer with SHA 'some_nonexistent_sha' not found") + h.AssertError(t, err, "failed to find cache layer with SHA 'some_nonexistent_sha'") }) }) }) @@ -230,7 +235,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { when("layer does not exist", func() { it("returns an error", func() { _, err := subject.RetrieveLayerFile("some_nonexistent_sha") - h.AssertError(t, err, "layer with SHA 'some_nonexistent_sha' not found") + h.AssertError(t, err, "failed to find cache layer with SHA 'some_nonexistent_sha'") }) }) }) @@ -340,7 +345,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.AddLayerFile(tarPath, "some_sha")) _, err := subject.RetrieveLayer("some_sha") - h.AssertError(t, err, "layer with SHA 'some_sha' not found") + h.AssertError(t, err, "failed to find cache layer with SHA 'some_sha'") }) }) @@ -415,7 +420,7 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.AddLayer(layerReader, layerSha)) _, err := subject.RetrieveLayer(layerSha) - h.AssertError(t, err, fmt.Sprintf("layer with SHA '%s' not found", layerSha)) + h.AssertError(t, err, fmt.Sprintf("failed to find cache layer with SHA '%s'", layerSha)) }) }) @@ -507,6 +512,21 @@ func testVolumeCache(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, string(bytes), "existing data") }) }) + + when("the layer does not exist", func() { + it("fails with a read error", func() { + err := subject.ReuseLayer("some_nonexistent_sha") + isReadErr, _ := cache.IsReadErr(err) + h.AssertEq(t, isReadErr, true) + + err = subject.Commit() + h.AssertNil(t, err) + + _, err = subject.RetrieveLayer("some_sha") + isReadErr, _ = cache.IsReadErr(err) + h.AssertEq(t, isReadErr, true) + }) + }) }) when("attempting to commit more than once", func() { diff --git a/cmd/lifecycle/exporter.go b/cmd/lifecycle/exporter.go index 305186617..06929fe53 100644 --- a/cmd/lifecycle/exporter.go +++ b/cmd/lifecycle/exporter.go @@ -19,6 +19,8 @@ import ( "github.com/pkg/errors" "golang.org/x/sync/errgroup" + "github.com/buildpacks/lifecycle/log" + "github.com/buildpacks/lifecycle/auth" "github.com/buildpacks/lifecycle/buildpack" "github.com/buildpacks/lifecycle/cache" @@ -200,7 +202,7 @@ func (e *exportCmd) export(group buildpack.Group, cacheStore phase.Cache, analyz case e.UseLayout: appImage, runImageID, err = e.initLayoutAppImage(analyzedMD) case e.UseDaemon: - appImage, runImageID, err = e.initDaemonAppImage(analyzedMD) + appImage, runImageID, err = e.initDaemonAppImage(analyzedMD, cmd.DefaultLogger) default: appImage, runImageID, err = e.initRemoteAppImage(analyzedMD) } @@ -258,7 +260,7 @@ func (e *exportCmd) export(group buildpack.Group, cacheStore phase.Cache, analyz return nil } -func (e *exportCmd) initDaemonAppImage(analyzedMD files.Analyzed) (imgutil.Image, string, error) { +func (e *exportCmd) initDaemonAppImage(analyzedMD files.Analyzed, logger log.Logger) (imgutil.Image, string, error) { var opts = []imgutil.ImageOption{ local.FromBaseImage(e.RunImageRef), } @@ -301,7 +303,7 @@ func (e *exportCmd) initDaemonAppImage(analyzedMD files.Analyzed) (imgutil.Image } if e.LaunchCacheDir != "" { - volumeCache, err := cache.NewVolumeCache(e.LaunchCacheDir) + volumeCache, err := cache.NewVolumeCache(e.LaunchCacheDir, logger) if err != nil { return nil, "", cmd.FailErr(err, "create launch cache") } diff --git a/cmd/lifecycle/main.go b/cmd/lifecycle/main.go index 74b51a010..42fede727 100644 --- a/cmd/lifecycle/main.go +++ b/cmd/lifecycle/main.go @@ -96,14 +96,14 @@ func (ch *DefaultCacheHandler) InitCache(cacheImageRef string, cacheDir string, cacheStore phase.Cache err error ) + logger := cmd.DefaultLogger if cacheImageRef != "" { - logger := cmd.DefaultLogger cacheStore, err = cache.NewImageCacheFromName(cacheImageRef, ch.keychain, logger, cache.NewImageDeleter(cache.NewImageComparer(), logger, deletionEnabled)) if err != nil { return nil, errors.Wrap(err, "creating image cache") } } else if cacheDir != "" { - cacheStore, err = cache.NewVolumeCache(cacheDir) + cacheStore, err = cache.NewVolumeCache(cacheDir, logger) if err != nil { return nil, errors.Wrap(err, "creating volume cache") } @@ -118,14 +118,14 @@ func initCache(cacheImageTag, cacheDir string, keychain authn.Keychain, deletion cacheStore phase.Cache err error ) + logger := cmd.DefaultLogger if cacheImageTag != "" { - logger := cmd.DefaultLogger cacheStore, err = cache.NewImageCacheFromName(cacheImageTag, keychain, logger, cache.NewImageDeleter(cache.NewImageComparer(), logger, deletionEnabled)) if err != nil { return nil, cmd.FailErr(err, "create image cache") } } else if cacheDir != "" { - cacheStore, err = cache.NewVolumeCache(cacheDir) + cacheStore, err = cache.NewVolumeCache(cacheDir, logger) if err != nil { return nil, cmd.FailErr(err, "create volume cache") } diff --git a/internal/layer/sbom_restorer_test.go b/internal/layer/sbom_restorer_test.go index 544b3f296..f542f00ed 100644 --- a/internal/layer/sbom_restorer_test.go +++ b/internal/layer/sbom_restorer_test.go @@ -164,7 +164,7 @@ func testSBOMRestorer(t *testing.T, when spec.G, it spec.S) { cacheDir, err = os.MkdirTemp("", "lifecycle.cache-dir.") h.AssertNil(t, err) - testCache, err = cache.NewVolumeCache(cacheDir) + testCache, err = cache.NewVolumeCache(cacheDir, &log.Logger{Handler: &discard.Handler{}}) h.AssertNil(t, err) h.AssertNil(t, testCache.AddLayerFile(layer.TarPath, layer.Digest)) h.AssertNil(t, testCache.SetMetadata(platform.CacheMetadata{BOM: files.LayerMetadata{SHA: layer.Digest}})) diff --git a/phase/analyzer.go b/phase/analyzer.go index 946d5ba4c..256dac7a9 100644 --- a/phase/analyzer.go +++ b/phase/analyzer.go @@ -42,7 +42,7 @@ func (f *ConnectedFactory) NewAnalyzer(inputs platform.LifecycleInputs, logger l } var err error - if analyzer.PreviousImage, err = f.getPreviousImage(inputs.PreviousImageRef, inputs.LaunchCacheDir); err != nil { + if analyzer.PreviousImage, err = f.getPreviousImage(inputs.PreviousImageRef, inputs.LaunchCacheDir, logger); err != nil { return nil, err } if analyzer.RunImage, err = f.getRunImage(inputs.RunImageRef); err != nil { diff --git a/phase/analyzer_test.go b/phase/analyzer_test.go index 0424f3f60..b9224bce0 100644 --- a/phase/analyzer_test.go +++ b/phase/analyzer_test.go @@ -332,6 +332,7 @@ func testAnalyzer(platformAPI string) func(t *testing.T, when spec.G, it spec.S) it.Before(func() { var err error + discardLogger := log.Logger{Handler: &discard.Handler{}} tmpDir, err = os.MkdirTemp("", "analyzer-tests") h.AssertNil(t, err) @@ -342,15 +343,13 @@ func testAnalyzer(platformAPI string) func(t *testing.T, when spec.G, it spec.S) cacheDir, err = os.MkdirTemp("", "some-cache-dir") h.AssertNil(t, err) - testCache, err = cache.NewVolumeCache(cacheDir) + testCache, err = cache.NewVolumeCache(cacheDir, &discardLogger) h.AssertNil(t, err) previousImage = fakes.NewImage("image-repo-name", "", local.IDIdentifier{ ImageID: "s0m3D1g3sT", }) - discardLogger := log.Logger{Handler: &discard.Handler{}} - mockCtrl = gomock.NewController(t) sbomRestorer = testmock.NewMockSBOMRestorer(mockCtrl) diff --git a/phase/cache.go b/phase/cache.go index 16685d97d..4467df0d8 100644 --- a/phase/cache.go +++ b/phase/cache.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/buildpacks/lifecycle/buildpack" + c "github.com/buildpacks/lifecycle/cache" "github.com/buildpacks/lifecycle/layers" "github.com/buildpacks/lifecycle/log" "github.com/buildpacks/lifecycle/platform" @@ -100,7 +101,14 @@ func (e *Exporter) addOrReuseCacheLayer(cache Cache, layerDir LayerDir, previous if layer.Digest == previousSHA { e.Logger.Infof("Reusing cache layer '%s'\n", layer.ID) e.Logger.Debugf("Layer '%s' SHA: %s\n", layer.ID, layer.Digest) - return layer.Digest, cache.ReuseLayer(previousSHA) + err = cache.ReuseLayer(previousSHA) + if err != nil { + isReadErr, readErr := c.IsReadErr(err) + if !isReadErr { + return "", errors.Wrapf(err, "reusing layer %s", layer.ID) + } + e.Logger.Warnf("Skipping re-use for layer %s: %s", layer.ID, readErr.Error()) + } } e.Logger.Infof("Adding cache layer '%s'\n", layer.ID) e.Logger.Debugf("Layer '%s' SHA: %s\n", layer.ID, layer.Digest) diff --git a/phase/cache_test.go b/phase/cache_test.go index 03971e7c9..bf73376a3 100644 --- a/phase/cache_test.go +++ b/phase/cache_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/apex/log" + "github.com/apex/log/handlers/discard" "github.com/apex/log/handlers/memory" "github.com/golang/mock/gomock" "github.com/sclevine/spec" @@ -46,6 +47,10 @@ func testCache(t *testing.T, when spec.G, it spec.S) { mockCtrl = gomock.NewController(t) layerFactory = testmock.NewMockLayerFactory(mockCtrl) + logHandler = memory.New() + level, err := log.ParseLevel("info") + h.AssertNil(t, err) + tmpDir, err = os.MkdirTemp("", "lifecycle.cacher.layer") h.AssertNil(t, err) h.AssertNil(t, os.Mkdir(filepath.Join(tmpDir, "artifacts"), 0777)) @@ -53,11 +58,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { cacheDir = filepath.Join(tmpDir, "cache") h.AssertNil(t, os.Mkdir(cacheDir, 0777)) - testCache, err = cache.NewVolumeCache(cacheDir) - h.AssertNil(t, err) - - logHandler = memory.New() - level, err := log.ParseLevel("info") + testCache, err = cache.NewVolumeCache(cacheDir, &log.Logger{Handler: logHandler, Level: level}) h.AssertNil(t, err) exporter = &phase.Exporter{ @@ -300,13 +301,15 @@ func assertCacheHasLayer(t *testing.T, cache phase.Cache, id string) { } func initializeCache(t *testing.T, exporter *phase.Exporter, testCache *phase.Cache, cacheDir, layersDir, metadataTemplate string) { - previousCache, err := cache.NewVolumeCache(cacheDir) + logger := &log.Logger{Handler: &discard.Handler{}} + + previousCache, err := cache.NewVolumeCache(cacheDir, logger) h.AssertNil(t, err) err = exporter.Cache(layersDir, previousCache) h.AssertNil(t, err) - *testCache, err = cache.NewVolumeCache(cacheDir) + *testCache, err = cache.NewVolumeCache(cacheDir, logger) h.AssertNil(t, err) h.AssertNil(t, os.WriteFile( diff --git a/phase/connected_factory.go b/phase/connected_factory.go index 3514938a2..083d4ca0a 100644 --- a/phase/connected_factory.go +++ b/phase/connected_factory.go @@ -5,6 +5,8 @@ import ( "github.com/buildpacks/imgutil" + "github.com/buildpacks/lifecycle/log" + "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/cache" "github.com/buildpacks/lifecycle/image" @@ -58,7 +60,7 @@ func (f *ConnectedFactory) ensureRegistryAccess(inputs platform.LifecycleInputs) return nil } -func (f *ConnectedFactory) getPreviousImage(imageRef string, launchCacheDir string) (imgutil.Image, error) { +func (f *ConnectedFactory) getPreviousImage(imageRef string, launchCacheDir string, logger log.Logger) (imgutil.Image, error) { if imageRef == "" { return nil, nil } @@ -69,7 +71,7 @@ func (f *ConnectedFactory) getPreviousImage(imageRef string, launchCacheDir stri if launchCacheDir == "" || f.imageHandler.Kind() != image.LocalKind { return previousImage, nil } - volumeCache, err := cache.NewVolumeCache(launchCacheDir) + volumeCache, err := cache.NewVolumeCache(launchCacheDir, logger) if err != nil { return nil, fmt.Errorf("creating launch cache: %w", err) } diff --git a/phase/restorer.go b/phase/restorer.go index e6e6ee6f5..901c28124 100644 --- a/phase/restorer.go +++ b/phase/restorer.go @@ -6,6 +6,8 @@ import ( "github.com/pkg/errors" "golang.org/x/sync/errgroup" + c "github.com/buildpacks/lifecycle/cache" + "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/buildpack" "github.com/buildpacks/lifecycle/internal/layer" @@ -98,7 +100,16 @@ func (r *Restorer) Restore(cache Cache) error { } else { r.Logger.Infof("Restoring data for %q from cache", bpLayer.Identifier()) g.Go(func() error { - return r.restoreCacheLayer(cache, cachedLayer.SHA) + err = r.restoreCacheLayer(cache, cachedLayer.SHA) + if err != nil { + isReadErr, readErr := c.IsReadErr(err) + if isReadErr { + r.Logger.Warnf("Skipping restore for layer %s: %s", bpLayer.Identifier(), readErr.Error()) + return nil + } + return errors.Wrapf(err, "restoring layer %s", bpLayer.Identifier()) + } + return nil }) } } diff --git a/phase/restorer_test.go b/phase/restorer_test.go index 94b3e1654..9f4b4a5e6 100644 --- a/phase/restorer_test.go +++ b/phase/restorer_test.go @@ -54,6 +54,8 @@ func testRestorer(buildpackAPI, platformAPI string) func(t *testing.T, when spec it.Before(func() { var err error + logHandler = memory.New() + logger := log.Logger{Handler: logHandler, Level: log.DebugLevel} layersDir, err = os.MkdirTemp("", "lifecycle-layer-dir") h.AssertNil(t, err) @@ -61,13 +63,9 @@ func testRestorer(buildpackAPI, platformAPI string) func(t *testing.T, when spec cacheDir, err = os.MkdirTemp("", "") h.AssertNil(t, err) - testCache, err = cache.NewVolumeCache(cacheDir) + testCache, err = cache.NewVolumeCache(cacheDir, &logger) h.AssertNil(t, err) - logHandler = memory.New() - - logger := log.Logger{Handler: logHandler, Level: log.DebugLevel} - mockCtrl = gomock.NewController(t) sbomRestorer = testmock.NewMockSBOMRestorer(mockCtrl) if api.MustParse(platformAPI).AtLeast("0.8") { @@ -203,53 +201,7 @@ func testRestorer(buildpackAPI, platformAPI string) func(t *testing.T, when spec h.AssertNil(t, os.RemoveAll(layersDir)) h.AssertNil(t, os.Mkdir(layersDir, 0777)) - contents := fmt.Sprintf(`{ - "buildpacks": [ - { - "key": "buildpack.id", - "layers": { - "cache-false": { - "cache": false, - "sha": "%s" - }, - "cache-launch": { - "cache": true, - "launch": true, - "sha": "%s" - }, - "cache-only": { - "cache": true, - "data": { - "cache-only-key": "cache-only-val" - }, - "sha": "%s" - } - } - }, - { - "key": "nogroup.buildpack.id", - "layers": { - "some-layer": { - "cache": true, - "sha": "%s" - } - } - }, - { - "key": "escaped/buildpack/id", - "layers": { - "escaped-bp-layer": { - "cache": true, - "data": { - "escaped-bp-key": "escaped-bp-val" - }, - "sha": "%s" - } - } - } - ] -} -`, cacheFalseLayerSHA, cacheLaunchLayerSHA, cacheOnlyLayerSHA, noGroupLayerSHA, escapedLayerSHA) + contents := buildMetadata(cacheFalseLayerSHA, cacheLaunchLayerSHA, cacheOnlyLayerSHA, noGroupLayerSHA, escapedLayerSHA) err = os.WriteFile( filepath.Join(cacheDir, "committed", "io.buildpacks.lifecycle.cache.metadata"), @@ -348,6 +300,40 @@ func testRestorer(buildpackAPI, platformAPI string) func(t *testing.T, when spec }) }) + when("there is a cache-only layer referenced in metadata that does not exist", func() { + var nonExistentCacheLaunchLayerSHA string + + it.Before(func() { + nonExistentCacheLaunchLayerSHA = "some-made-up-sha" + contents := buildMetadata(cacheFalseLayerSHA, nonExistentCacheLaunchLayerSHA, cacheOnlyLayerSHA, noGroupLayerSHA, escapedLayerSHA) + + err := os.WriteFile( + filepath.Join(cacheDir, "committed", "io.buildpacks.lifecycle.cache.metadata"), + []byte(contents), + 0600, + ) + h.AssertNil(t, err) + + err = restorer.Restore(testCache) + h.AssertNil(t, err) + }) + + it("restores expected cache-only layer", func() { + got := h.MustReadFile(t, filepath.Join(layersDir, "buildpack.id", "cache-only", "file-from-cache-only-layer")) + want := "echo text from cache-only layer\n" + h.AssertEq(t, string(got), want) + }) + + it("keeps expected layer metadata", func() { + got := h.MustReadFile(t, filepath.Join(layersDir, "buildpack.id", "cache-only.toml")) + h.AssertEq(t, string(got), "[metadata]\n cache-only-key = \"cache-only-val\"\n") + }) + + it("skips restoring non-existent cache-launch layer", func() { + h.AssertPathDoesNotExist(t, filepath.Join(layersDir, "buildpack.id", "cache-launch")) + }) + }) + when("there is a cache=true layer not in cache", func() { it.Before(func() { var meta, sha string @@ -550,3 +536,53 @@ func TestWriteLayer(t *testing.T) { h.AssertPathDoesNotExist(t, filepath.Join(layersDir, "test-buildpack", "test-layer")) } + +func buildMetadata(cacheFalseLayerSHA string, cacheLaunchLayerSHA string, cacheOnlyLayerSHA string, noGroupLayerSHA string, escapedLayerSHA string) string { + return fmt.Sprintf(`{ + "buildpacks": [ + { + "key": "buildpack.id", + "layers": { + "cache-false": { + "cache": false, + "sha": "%s" + }, + "cache-launch": { + "cache": true, + "launch": true, + "sha": "%s" + }, + "cache-only": { + "cache": true, + "data": { + "cache-only-key": "cache-only-val" + }, + "sha": "%s" + } + } + }, + { + "key": "nogroup.buildpack.id", + "layers": { + "some-layer": { + "cache": true, + "sha": "%s" + } + } + }, + { + "key": "escaped/buildpack/id", + "layers": { + "escaped-bp-layer": { + "cache": true, + "data": { + "escaped-bp-key": "escaped-bp-val" + }, + "sha": "%s" + } + } + } + ] +} +`, cacheFalseLayerSHA, cacheLaunchLayerSHA, cacheOnlyLayerSHA, noGroupLayerSHA, escapedLayerSHA) +} diff --git a/phase/testmock/cache/image_deleter.go b/phase/testmock/cache/image_deleter.go index f4a6051ec..39d6a4b5f 100644 --- a/phase/testmock/cache/image_deleter.go +++ b/phase/testmock/cache/image_deleter.go @@ -34,6 +34,18 @@ func (m *MockImageDeleter) EXPECT() *MockImageDeleterMockRecorder { return m.recorder } +// DeleteImage mocks base method. +func (m *MockImageDeleter) DeleteImage(arg0 imgutil.Image) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteImage", arg0) +} + +// DeleteImage indicates an expected call of DeleteImage. +func (mr *MockImageDeleterMockRecorder) DeleteImage(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteImage", reflect.TypeOf((*MockImageDeleter)(nil).DeleteImage), arg0) +} + // DeleteOrigImageIfDifferentFromNewImage mocks base method. func (m *MockImageDeleter) DeleteOrigImageIfDifferentFromNewImage(arg0, arg1 imgutil.Image) { m.ctrl.T.Helper()