Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corrupted cache #16

Merged
merged 38 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
37d6985
Bump imgutil to pick up fix from https://github.com/buildpacks/imguti…
natalieparellano Apr 22, 2024
13d3691
Try to fix test-s390x workflow (#1339)
natalieparellano Apr 29, 2024
44b7041
When a buildpack declares distro information, but a base image does n…
natalieparellano May 1, 2024
effdf24
Read `/etc/os-release` file when distro information is not present in…
pbusko May 6, 2024
8a14842
Bump the go-dependencies group across 1 directory with 5 updates (#1349)
dependabot[bot] May 6, 2024
c0590cd
Bump the go-dependencies group with 2 updates (#1351)
dependabot[bot] May 8, 2024
83efa75
Also read distro information from /etc/os-release when checking targe…
natalieparellano May 10, 2024
1a1de08
fix: ibmcloud CLI nic attachment issue for s390x testing (#1353)
dilipgb May 13, 2024
73f6927
More fixes for target compat checking during detect (#1354)
natalieparellano May 14, 2024
85b745c
Bump azure/docker-login from 1 to 2 (#1359)
dependabot[bot] Jun 5, 2024
36c0af0
Bump imgutil to pick up fixes for containerd and podman (#1361)
natalieparellano Jun 5, 2024
7b5a8ec
Bump the go-dependencies group across 1 directory with 5 updates (#1360)
dependabot[bot] Jun 5, 2024
9e19760
add failing test to restorer
joeybrown-sf May 9, 2024
376931a
restorer and exporter working as expected
joeybrown-sf May 8, 2024
1c4aefa
lint
joeybrown-sf Jul 2, 2024
a02be03
Ensure read access to the run image selected by extensions (#1364)
pbusko Jul 3, 2024
f2a3bd7
Restore cached launch layers not found in `appLayers` (#1346)
pbusko Jul 3, 2024
0101e33
Update phase/restorer.go
joeybrown-sf Jul 9, 2024
3d30c91
Update phase/cache.go
joeybrown-sf Jul 9, 2024
8570898
Update cache/image_cache.go
joeybrown-sf Jul 9, 2024
2c58c8d
Update cache/volume_cache.go
joeybrown-sf Jul 9, 2024
55aa264
Update cache/volume_cache.go
joeybrown-sf Jul 9, 2024
de0806a
Update cache/volume_cache.go
joeybrown-sf Jul 9, 2024
1c304d5
Update cache/volume_cache.go
joeybrown-sf Jul 9, 2024
80d089f
Update cache/volume_cache.go
joeybrown-sf Jul 9, 2024
6a2a590
update based on feedback
joeybrown-sf Jul 9, 2024
dcea1b3
fix log
joeybrown-sf Jul 9, 2024
04f1ad1
Fix CNB_TARGET_* env vars on older Platform API (#1374)
natalieparellano Jul 9, 2024
4c40dca
Bump the go-dependencies group across 1 directory with 6 updates (#1373)
dependabot[bot] Jul 9, 2024
12e2de8
Bump google.golang.org/grpc from 1.64.0 to 1.64.1 (#1375)
dependabot[bot] Jul 10, 2024
a87e12e
Surface registry error (#1376)
natalieparellano Jul 11, 2024
dba6b9a
temp fix
joeybrown-sf Jul 12, 2024
67a8cf5
this does not work as is. I think we need to modify img utils.
joeybrown-sf Jul 12, 2024
1b5aa7c
add eof check
joeybrown-sf Jul 15, 2024
6a779e3
add not exist check
joeybrown-sf Jul 16, 2024
ba02977
reuse layer test
joeybrown-sf Jul 16, 2024
289442a
fix test regression
joeybrown-sf Jul 16, 2024
39f37e7
Merge branch 'main' into corrupted-cache
natalieparellano Jul 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cache/caching_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down
22 changes: 22 additions & 0 deletions cache/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
33 changes: 31 additions & 2 deletions cache/image_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
joeybrown-sf marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down
4 changes: 2 additions & 2 deletions cache/image_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'")
})
})
})
Expand Down Expand Up @@ -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))
})
})
})
Expand Down
7 changes: 4 additions & 3 deletions cache/image_deleter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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())
Expand Down
33 changes: 29 additions & 4 deletions cache/volume_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cache

import (
"encoding/json"
"fmt"
"io"
"os"
"path/filepath"
Expand All @@ -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"
)
Expand All @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
natalieparellano marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -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) {
joeybrown-sf marked this conversation as resolved.
Show resolved Hide resolved
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
}
Expand All @@ -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)
}
Expand Down
40 changes: 30 additions & 10 deletions cache/volume_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand All @@ -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() {
Expand All @@ -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")
}
Expand All @@ -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"))
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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"))
Expand All @@ -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)
})

Expand Down Expand Up @@ -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'")
})
})
})
Expand All @@ -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'")
})
})
})
Expand Down Expand Up @@ -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'")
})
})

Expand Down Expand Up @@ -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))
})
})

Expand Down Expand Up @@ -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() {
Expand Down
8 changes: 5 additions & 3 deletions cmd/lifecycle/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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),
}
Expand Down Expand Up @@ -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")
}
Expand Down
Loading
Loading