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.

Sign up for GitHub

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

Jump to bottom

executor error improvements #5179

Merged
merged 3 commits into from
Aug 9, 2024
Merged

executor error improvements #5179

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7720ed
executor: ensure deeper stacktraces for system errors
tonistiigi Jul 22, 2024
2f8ab30
executor: rebase the path of submount error
tonistiigi Jul 22, 2024
304b6ec
ops: improve error messages from fileop
tonistiigi Jul 22, 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
72 changes: 52 additions & 20 deletions solver/llbsolver/file/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,17 @@ func timestampToTime(ts int64) *time.Time {
return &tm
}

func mkdir(d string, action pb.FileActionMkDir, user *copy.User, idmap *idtools.IdentityMapping) error {
func mkdir(d string, action pb.FileActionMkDir, user *copy.User, idmap *idtools.IdentityMapping) (err error) {
defer func() {
var osErr *os.PathError
if errors.As(err, &osErr) {
osErr.Path = strings.TrimPrefix(osErr.Path, d)
}
}()

p, err := fs.RootPath(d, action.Path)
if err != nil {
return err
return errors.WithStack(err)
}

ch, err := mapUserToChowner(user, idmap)
Expand All @@ -47,23 +54,31 @@ func mkdir(d string, action pb.FileActionMkDir, user *copy.User, idmap *idtools.
if errors.Is(err, os.ErrExist) {
return nil
}
return err
return errors.WithStack(err)
}
if err := copy.Chown(p, nil, ch); err != nil {
return err
return errors.WithStack(err)
}
if err := copy.Utimes(p, timestampToTime(action.Timestamp)); err != nil {
return err
return errors.WithStack(err)
}
}

return nil
}

func mkfile(d string, action pb.FileActionMkFile, user *copy.User, idmap *idtools.IdentityMapping) error {
func mkfile(d string, action pb.FileActionMkFile, user *copy.User, idmap *idtools.IdentityMapping) (err error) {
defer func() {
var osErr *os.PathError
if errors.As(err, &osErr) {
// remove system root from error path if present
osErr.Path = strings.TrimPrefix(osErr.Path, d)
}
}()

p, err := fs.RootPath(d, filepath.Join("/", action.Path))
if err != nil {
return err
return errors.WithStack(err)
}

ch, err := mapUserToChowner(user, idmap)
Expand All @@ -72,29 +87,37 @@ func mkfile(d string, action pb.FileActionMkFile, user *copy.User, idmap *idtool
}

if err := os.WriteFile(p, action.Data, os.FileMode(action.Mode)&0777); err != nil {
return err
return errors.WithStack(err)
}

if err := copy.Chown(p, nil, ch); err != nil {
return err
return errors.WithStack(err)
}

if err := copy.Utimes(p, timestampToTime(action.Timestamp)); err != nil {
return err
return errors.WithStack(err)
}

return nil
}

func rm(d string, action pb.FileActionRm) error {
func rm(d string, action pb.FileActionRm) (err error) {
defer func() {
var osErr *os.PathError
if errors.As(err, &osErr) {
// remove system root from error path if present
osErr.Path = strings.TrimPrefix(osErr.Path, d)
}
}()

if action.AllowWildcard {
src, err := cleanPath(action.Path)
if err != nil {
return errors.Wrap(err, "cleaning path")
}
m, err := copy.ResolveWildcards(d, src, false)
if err != nil {
return err
return errors.WithStack(err)
}

for _, s := range m {
Expand All @@ -117,22 +140,22 @@ func rmPath(root, src string, allowNotFound bool) error {
}
dir, err := fs.RootPath(root, filepath.Join("/", dir))
if err != nil {
return err
return errors.WithStack(err)
}
p := filepath.Join(dir, base)

if !allowNotFound {
_, err := os.Stat(p)

if errors.Is(err, os.ErrNotExist) {
return err
return errors.WithStack(err)
}
}

return os.RemoveAll(p)
return errors.WithStack(os.RemoveAll(p))
}

func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u *copy.User, idmap *idtools.IdentityMapping) error {
func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u *copy.User, idmap *idtools.IdentityMapping) (err error) {
srcPath, err := cleanPath(action.Src)
if err != nil {
return errors.Wrap(err, "cleaning source path")
Expand All @@ -144,7 +167,7 @@ func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u *
if !action.CreateDestPath {
p, err := fs.RootPath(dest, filepath.Join("/", action.Dest))
if err != nil {
return err
return errors.WithStack(err)
}
if _, err := os.Lstat(filepath.Dir(p)); err != nil {
return errors.Wrapf(err, "failed to stat %s", action.Dest)
Expand Down Expand Up @@ -177,14 +200,23 @@ func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u *
copy.WithXAttrErrorHandler(xattrErrorHandler),
}

defer func() {
var osErr *os.PathError
if errors.As(err, &osErr) {
// remove system root from error path if present
osErr.Path = strings.TrimPrefix(osErr.Path, src)
osErr.Path = strings.TrimPrefix(osErr.Path, dest)
thompson-shaun marked this conversation as resolved.
Show resolved Hide resolved
}
}()

var m []string
if !action.AllowWildcard {
m = []string{srcPath}
} else {
var err error
m, err = copy.ResolveWildcards(src, srcPath, action.FollowSymlink)
if err != nil {
return err
return errors.WithStack(err)
}

if len(m) == 0 {
Expand All @@ -198,13 +230,13 @@ func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u *
for _, s := range m {
if action.AttemptUnpackDockerCompatibility {
if ok, err := unpack(src, s, dest, destPath, ch, timestampToTime(action.Timestamp), idmap); err != nil {
return err
return errors.WithStack(err)
} else if ok {
continue
}
}
if err := copy.Copy(ctx, src, s, dest, destPath, opt...); err != nil {
return err
return errors.WithStack(err)
}
}

Expand Down
3 changes: 2 additions & 1 deletion solver/llbsolver/file/backend_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ import (
"path/filepath"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/require"
)

func TestRmPathNonExistentFileAllowNotFoundFalse(t *testing.T) {
root := t.TempDir()
err := rmPath(root, "doesnt_exist", false)
require.Error(t, err)
require.True(t, os.IsNotExist(err))
require.True(t, errors.Is(err, os.ErrNotExist))
}

func TestRmPathNonExistentFileAllowNotFoundTrue(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions solver/llbsolver/file/backend_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package file

import (
"github.com/docker/docker/pkg/idtools"
"github.com/pkg/errors"
copy "github.com/tonistiigi/fsutil/copy"
)

Expand All @@ -23,7 +24,7 @@ func mapUserToChowner(user *copy.User, idmap *idtools.IdentityMapping) (copy.Cho
GID: old.GID,
})
if err != nil {
return nil, err
return nil, errors.WithStack(err)
}
return &copy.User{UID: identity.UID, GID: identity.GID}, nil
}
Expand All @@ -38,7 +39,7 @@ func mapUserToChowner(user *copy.User, idmap *idtools.IdentityMapping) (copy.Cho
GID: user.GID,
})
if err != nil {
return nil, err
return nil, errors.WithStack(err)
}
u.UID = identity.UID
u.GID = identity.GID
Expand Down
10 changes: 5 additions & 5 deletions solver/llbsolver/ops/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (f *fileOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu

backend, err := file.NewFileOpBackend(getReadUserFn(f.w))
if err != nil {
return nil, err
return nil, errors.WithStack(err)
}

fs := NewFileOpSolver(f.w, backend, f.refManager)
Expand Down Expand Up @@ -474,7 +474,7 @@ func (s *FileOpSolver) getInput(ctx context.Context, idx int, inputs []fileoptyp
if inp.ref != nil {
m, err := s.r.Prepare(ctx, inp.ref, false, g)
if err != nil {
return err
return errors.WithStack(err)
}
inpMount = m
return nil
Expand All @@ -493,7 +493,7 @@ func (s *FileOpSolver) getInput(ctx context.Context, idx int, inputs []fileoptyp
if inp.ref != nil {
m, err := s.r.Prepare(ctx, inp.ref, true, g)
if err != nil {
return err
return errors.WithStack(err)
}
inpMountSecondary = m
toRelease = append(toRelease, m)
Expand Down Expand Up @@ -521,7 +521,7 @@ func (s *FileOpSolver) getInput(ctx context.Context, idx int, inputs []fileoptyp
if inp.ref != nil {
mm, err := s.r.Prepare(ctx, inp.ref, true, g)
if err != nil {
return nil, err
return nil, errors.WithStack(err)
}
toRelease = append(toRelease, mm)
m = mm
Expand Down Expand Up @@ -572,7 +572,7 @@ func (s *FileOpSolver) getInput(ctx context.Context, idx int, inputs []fileoptyp
if inpMount == nil {
m, err := s.r.Prepare(ctx, nil, false, g)
if err != nil {
return input{}, err
return input{}, errors.WithStack(err)
}
inpMount = m
}
Expand Down
Loading