From ed6bbb2bf57898e4341b6ff8ee3b04cb5cd21c8a Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sat, 1 Jun 2019 16:34:02 -0700 Subject: [PATCH 1/6] cache: add more error tracing Signed-off-by: Tonis Tiigi --- cache/manager.go | 14 +++++----- cache/metadata/metadata.go | 53 +++++++++++++++++++------------------- cache/refs.go | 4 +-- 3 files changed, 35 insertions(+), 36 deletions(-) diff --git a/cache/manager.go b/cache/manager.go index e3522f659..0c7ec789e 100644 --- a/cache/manager.go +++ b/cache/manager.go @@ -157,14 +157,14 @@ func (cm *cacheManager) get(ctx context.Context, id string, fromSnapshotter bool func (cm *cacheManager) getRecord(ctx context.Context, id string, fromSnapshotter bool, opts ...RefOption) (cr *cacheRecord, retErr error) { if rec, ok := cm.records[id]; ok { if rec.isDead() { - return nil, errNotFound + return nil, errors.Wrapf(errNotFound, "failed to get dead record %s", id) } return rec, nil } md, ok := cm.md.Get(id) if !ok && !fromSnapshotter { - return nil, errNotFound + return nil, errors.WithStack(errNotFound) } if mutableID := getEqualMutable(md); mutableID != "" { mutable, err := cm.getRecord(ctx, mutableID, fromSnapshotter) @@ -222,7 +222,7 @@ func (cm *cacheManager) getRecord(ctx context.Context, id string, fromSnapshotte if err := rec.remove(ctx, true); err != nil { return nil, err } - return nil, errNotFound + return nil, errors.Wrapf(errNotFound, "failed to get deleted record %s", id) } if err := initializeMetadata(rec, opts...); err != nil { @@ -330,14 +330,14 @@ func (cm *cacheManager) Prune(ctx context.Context, ch chan client.UsageInfo, opt func (cm *cacheManager) pruneOnce(ctx context.Context, ch chan client.UsageInfo, opt client.PruneInfo) error { filter, err := filters.ParseAll(opt.Filter...) if err != nil { - return err + return errors.Wrapf(err, "failed to parse prune filters %v", opt.Filter) } var check ExternalRefChecker if f := cm.PruneRefChecker; f != nil && (!opt.All || len(opt.Filter) > 0) { c, err := f() if err != nil { - return err + return errors.WithStack(err) } check = c } @@ -549,7 +549,7 @@ func (cm *cacheManager) markShared(m map[string]*cacheUsageInfo) error { } c, err := cm.PruneRefChecker() if err != nil { - return err + return errors.WithStack(err) } var markAllParentsShared func(string) @@ -590,7 +590,7 @@ type cacheUsageInfo struct { func (cm *cacheManager) DiskUsage(ctx context.Context, opt client.DiskUsageInfo) ([]*client.UsageInfo, error) { filter, err := filters.ParseAll(opt.Filter...) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "failed to parse diskusage filters %v", opt.Filter) } cm.mu.Lock() diff --git a/cache/metadata/metadata.go b/cache/metadata/metadata.go index 9da270b4e..f43da0015 100644 --- a/cache/metadata/metadata.go +++ b/cache/metadata/metadata.go @@ -55,7 +55,7 @@ func (s *Store) All() ([]*StorageItem, error) { return nil }) }) - return out, err + return out, errors.WithStack(err) } func (s *Store) Probe(index string) (bool, error) { @@ -77,7 +77,7 @@ func (s *Store) Probe(index string) (bool, error) { } return nil }) - return exists, err + return exists, errors.WithStack(err) } func (s *Store) Search(index string) ([]*StorageItem, error) { @@ -114,7 +114,7 @@ func (s *Store) Search(index string) ([]*StorageItem, error) { } return nil }) - return out, err + return out, errors.WithStack(err) } func (s *Store) View(id string, fn func(b *bolt.Bucket) error) error { @@ -132,7 +132,7 @@ func (s *Store) View(id string, fn func(b *bolt.Bucket) error) error { } func (s *Store) Clear(id string) error { - return s.db.Update(func(tx *bolt.Tx) error { + return errors.WithStack(s.db.Update(func(tx *bolt.Tx) error { external := tx.Bucket([]byte(externalBucket)) if external != nil { external.DeleteBucket([]byte(id)) @@ -160,21 +160,21 @@ func (s *Store) Clear(id string) error { } } return main.DeleteBucket([]byte(id)) - }) + })) } func (s *Store) Update(id string, fn func(b *bolt.Bucket) error) error { - return s.db.Update(func(tx *bolt.Tx) error { + return errors.WithStack(s.db.Update(func(tx *bolt.Tx) error { b, err := tx.CreateBucketIfNotExists([]byte(mainBucket)) if err != nil { - return err + return errors.WithStack(err) } b, err = b.CreateBucketIfNotExists([]byte(id)) if err != nil { - return err + return errors.WithStack(err) } return fn(b) - }) + })) } func (s *Store) Get(id string) (*StorageItem, bool) { @@ -200,7 +200,7 @@ func (s *Store) Get(id string) (*StorageItem, bool) { } func (s *Store) Close() error { - return s.db.Close() + return errors.WithStack(s.db.Close()) } type StorageItem struct { @@ -222,13 +222,13 @@ func newStorageItem(id string, b *bolt.Bucket, s *Store) (*StorageItem, error) { var sv Value if len(v) > 0 { if err := json.Unmarshal(v, &sv); err != nil { - return err + return errors.WithStack(err) } si.values[string(k)] = &sv } return nil }); err != nil { - return si, err + return si, errors.WithStack(err) } } return si, nil @@ -283,23 +283,23 @@ func (s *StorageItem) GetExternal(k string) ([]byte, error) { return nil }) if err != nil { - return nil, err + return nil, errors.WithStack(err) } return dt, nil } func (s *StorageItem) SetExternal(k string, dt []byte) error { - return s.storage.db.Update(func(tx *bolt.Tx) error { + return errors.WithStack(s.storage.db.Update(func(tx *bolt.Tx) error { b, err := tx.CreateBucketIfNotExists([]byte(externalBucket)) if err != nil { - return err + return errors.WithStack(err) } b, err = b.CreateBucketIfNotExists([]byte(s.id)) if err != nil { - return err + return errors.WithStack(err) } return b.Put([]byte(k), dt) - }) + })) } func (s *StorageItem) Queue(fn func(b *bolt.Bucket) error) { @@ -311,15 +311,15 @@ func (s *StorageItem) Queue(fn func(b *bolt.Bucket) error) { func (s *StorageItem) Commit() error { s.mu.Lock() defer s.mu.Unlock() - return s.Update(func(b *bolt.Bucket) error { + return errors.WithStack(s.Update(func(b *bolt.Bucket) error { for _, fn := range s.queue { if err := fn(b); err != nil { - return err + return errors.WithStack(err) } } s.queue = s.queue[:0] return nil - }) + })) } func (s *StorageItem) Indexes() (out []string) { @@ -341,18 +341,18 @@ func (s *StorageItem) SetValue(b *bolt.Bucket, key string, v *Value) error { } dt, err := json.Marshal(v) if err != nil { - return err + return errors.WithStack(err) } if err := b.Put([]byte(key), dt); err != nil { - return err + return errors.WithStack(err) } if v.Index != "" { b, err := b.Tx().CreateBucketIfNotExists([]byte(indexBucket)) if err != nil { - return err + return errors.WithStack(err) } if err := b.Put([]byte(indexKey(v.Index, s.ID())), []byte{}); err != nil { - return err + return errors.WithStack(err) } } s.values[key] = v @@ -367,14 +367,13 @@ type Value struct { func NewValue(v interface{}) (*Value, error) { dt, err := json.Marshal(v) if err != nil { - return nil, err + return nil, errors.WithStack(err) } return &Value{Value: json.RawMessage(dt)}, nil } func (v *Value) Unmarshal(target interface{}) error { - err := json.Unmarshal(v.Value, target) - return err + return errors.WithStack(json.Unmarshal(v.Value, target)) } func indexKey(index, target string) string { diff --git a/cache/refs.go b/cache/refs.go index 63d46f2b8..ca839c01d 100644 --- a/cache/refs.go +++ b/cache/refs.go @@ -190,7 +190,7 @@ func (cr *cacheRecord) remove(ctx context.Context, removeSnapshot bool) error { } if removeSnapshot { if err := cr.cm.Snapshotter.Remove(ctx, cr.ID()); err != nil { - return err + return errors.Wrapf(err, "failed to remove %s", cr.ID()) } } if err := cr.cm.md.Clear(cr.ID()); err != nil { @@ -259,7 +259,7 @@ func (sr *immutableRef) release(ctx context.Context) error { if len(sr.refs) == 0 { if sr.viewMount != nil { // TODO: release viewMount earlier if possible if err := sr.cm.Snapshotter.Remove(ctx, sr.view); err != nil { - return err + return errors.Wrapf(err, "failed to remove view %s", sr.view) } sr.view = "" sr.viewMount = nil From d3597181e0d2f44682bf3df781c7423fffa5b327 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sat, 1 Jun 2019 16:52:35 -0700 Subject: [PATCH 2/6] session: wrap errors with debug info Make sure to cover the grpc errors origins. Signed-off-by: Tonis Tiigi --- session/auth/auth.go | 3 ++- session/content/caller.go | 25 ++++++++++++++++--------- session/filesync/diffcopy.go | 24 ++++++++++++------------ session/filesync/filesync.go | 4 ++-- session/secrets/secrets.go | 2 +- session/sshforward/copy.go | 9 +++++---- session/sshforward/ssh.go | 13 +++++++------ session/upload/upload.go | 7 ++++--- 8 files changed, 49 insertions(+), 38 deletions(-) diff --git a/session/auth/auth.go b/session/auth/auth.go index 2b96a7cef..f2cfa6699 100644 --- a/session/auth/auth.go +++ b/session/auth/auth.go @@ -4,6 +4,7 @@ import ( "context" "github.com/moby/buildkit/session" + "github.com/pkg/errors" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -19,7 +20,7 @@ func CredentialsFunc(ctx context.Context, c session.Caller) func(string) (string if st, ok := status.FromError(err); ok && st.Code() == codes.Unimplemented { return "", "", nil } - return "", "", err + return "", "", errors.WithStack(err) } return resp.Username, resp.Secret, nil } diff --git a/session/content/caller.go b/session/content/caller.go index ef7a24ec7..70e82130d 100644 --- a/session/content/caller.go +++ b/session/content/caller.go @@ -9,6 +9,7 @@ import ( "github.com/moby/buildkit/session" digest "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" "google.golang.org/grpc/metadata" ) @@ -31,47 +32,53 @@ func (cs *callerContentStore) choose(ctx context.Context) context.Context { func (cs *callerContentStore) Info(ctx context.Context, dgst digest.Digest) (content.Info, error) { ctx = cs.choose(ctx) - return cs.store.Info(ctx, dgst) + info, err := cs.store.Info(ctx, dgst) + return info, errors.WithStack(err) } func (cs *callerContentStore) Update(ctx context.Context, info content.Info, fieldpaths ...string) (content.Info, error) { ctx = cs.choose(ctx) - return cs.store.Update(ctx, info, fieldpaths...) + info, err := cs.store.Update(ctx, info, fieldpaths...) + return info, errors.WithStack(err) } func (cs *callerContentStore) Walk(ctx context.Context, fn content.WalkFunc, fs ...string) error { ctx = cs.choose(ctx) - return cs.store.Walk(ctx, fn, fs...) + return errors.WithStack(cs.store.Walk(ctx, fn, fs...)) } func (cs *callerContentStore) Delete(ctx context.Context, dgst digest.Digest) error { ctx = cs.choose(ctx) - return cs.store.Delete(ctx, dgst) + return errors.WithStack(cs.store.Delete(ctx, dgst)) } func (cs *callerContentStore) ListStatuses(ctx context.Context, fs ...string) ([]content.Status, error) { ctx = cs.choose(ctx) - return cs.store.ListStatuses(ctx, fs...) + resp, err := cs.store.ListStatuses(ctx, fs...) + return resp, errors.WithStack(err) } func (cs *callerContentStore) Status(ctx context.Context, ref string) (content.Status, error) { ctx = cs.choose(ctx) - return cs.store.Status(ctx, ref) + st, err := cs.store.Status(ctx, ref) + return st, errors.WithStack(err) } func (cs *callerContentStore) Abort(ctx context.Context, ref string) error { ctx = cs.choose(ctx) - return cs.store.Abort(ctx, ref) + return errors.WithStack(cs.store.Abort(ctx, ref)) } func (cs *callerContentStore) Writer(ctx context.Context, opts ...content.WriterOpt) (content.Writer, error) { ctx = cs.choose(ctx) - return cs.store.Writer(ctx, opts...) + w, err := cs.store.Writer(ctx, opts...) + return w, errors.WithStack(err) } func (cs *callerContentStore) ReaderAt(ctx context.Context, desc ocispec.Descriptor) (content.ReaderAt, error) { ctx = cs.choose(ctx) - return cs.store.ReaderAt(ctx, desc) + ra, err := cs.store.ReaderAt(ctx, desc) + return ra, errors.WithStack(err) } // NewCallerStore creates content.Store from session.Caller with specified storeID diff --git a/session/filesync/diffcopy.go b/session/filesync/diffcopy.go index 6934f9464..b82e3fc1c 100644 --- a/session/filesync/diffcopy.go +++ b/session/filesync/diffcopy.go @@ -14,7 +14,7 @@ import ( ) func sendDiffCopy(stream grpc.Stream, fs fsutil.FS, progress progressCb) error { - return fsutil.Send(stream.Context(), stream, fs, progress) + return errors.WithStack(fsutil.Send(stream.Context(), stream, fs, progress)) } func newStreamWriter(stream grpc.ClientStream) io.WriteCloser { @@ -29,7 +29,7 @@ type bufferedWriteCloser struct { func (bwc *bufferedWriteCloser) Close() error { if err := bwc.Writer.Flush(); err != nil { - return err + return errors.WithStack(err) } return bwc.Closer.Close() } @@ -40,19 +40,19 @@ type streamWriterCloser struct { func (wc *streamWriterCloser) Write(dt []byte) (int, error) { if err := wc.ClientStream.SendMsg(&BytesMessage{Data: dt}); err != nil { - return 0, err + return 0, errors.WithStack(err) } return len(dt), nil } func (wc *streamWriterCloser) Close() error { if err := wc.ClientStream.CloseSend(); err != nil { - return err + return errors.WithStack(err) } // block until receiver is done var bm BytesMessage if err := wc.ClientStream.RecvMsg(&bm); err != io.EOF { - return err + return errors.WithStack(err) } return nil } @@ -69,19 +69,19 @@ func recvDiffCopy(ds grpc.Stream, dest string, cu CacheUpdater, progress progres cf = cu.HandleChange ch = cu.ContentHasher() } - return fsutil.Receive(ds.Context(), ds, dest, fsutil.ReceiveOpt{ + return errors.WithStack(fsutil.Receive(ds.Context(), ds, dest, fsutil.ReceiveOpt{ NotifyHashed: cf, ContentHasher: ch, ProgressCb: progress, Filter: fsutil.FilterFunc(filter), - }) + })) } func syncTargetDiffCopy(ds grpc.Stream, dest string) error { if err := os.MkdirAll(dest, 0700); err != nil { - return err + return errors.Wrapf(err, "failed to create synctarget dest dir %s", dest) } - return fsutil.Receive(ds.Context(), ds, dest, fsutil.ReceiveOpt{ + return errors.WithStack(fsutil.Receive(ds.Context(), ds, dest, fsutil.ReceiveOpt{ Merge: true, Filter: func() func(string, *fstypes.Stat) bool { uid := os.Getuid() @@ -92,7 +92,7 @@ func syncTargetDiffCopy(ds grpc.Stream, dest string) error { return true } }(), - }) + })) } func writeTargetFile(ds grpc.Stream, wc io.WriteCloser) error { @@ -102,10 +102,10 @@ func writeTargetFile(ds grpc.Stream, wc io.WriteCloser) error { if errors.Cause(err) == io.EOF { return nil } - return err + return errors.WithStack(err) } if _, err := wc.Write(bm.Data); err != nil { - return err + return errors.WithStack(err) } } } diff --git a/session/filesync/filesync.go b/session/filesync/filesync.go index de5237b1f..b345569bf 100644 --- a/session/filesync/filesync.go +++ b/session/filesync/filesync.go @@ -275,7 +275,7 @@ func CopyToCaller(ctx context.Context, fs fsutil.FS, c session.Caller, progress cc, err := client.DiffCopy(ctx) if err != nil { - return err + return errors.WithStack(err) } return sendDiffCopy(cc, fs, progress) @@ -291,7 +291,7 @@ func CopyFileWriter(ctx context.Context, c session.Caller) (io.WriteCloser, erro cc, err := client.DiffCopy(ctx) if err != nil { - return nil, err + return nil, errors.WithStack(err) } return newStreamWriter(cc), nil diff --git a/session/secrets/secrets.go b/session/secrets/secrets.go index 6cfda18bb..1d27430d9 100644 --- a/session/secrets/secrets.go +++ b/session/secrets/secrets.go @@ -24,7 +24,7 @@ func GetSecret(ctx context.Context, c session.Caller, id string) ([]byte, error) if st, ok := status.FromError(err); ok && (st.Code() == codes.Unimplemented || st.Code() == codes.NotFound) { return nil, errors.Wrapf(ErrNotFound, "secret %s not found", id) } - return nil, err + return nil, errors.WithStack(err) } return resp.Data, nil } diff --git a/session/sshforward/copy.go b/session/sshforward/copy.go index c101f3b45..c2763fa45 100644 --- a/session/sshforward/copy.go +++ b/session/sshforward/copy.go @@ -3,6 +3,7 @@ package sshforward import ( io "io" + "github.com/pkg/errors" context "golang.org/x/net/context" "golang.org/x/sync/errgroup" "google.golang.org/grpc" @@ -19,7 +20,7 @@ func Copy(ctx context.Context, conn io.ReadWriteCloser, stream grpc.Stream) erro return nil } conn.Close() - return err + return errors.WithStack(err) } select { case <-ctx.Done(): @@ -29,7 +30,7 @@ func Copy(ctx context.Context, conn io.ReadWriteCloser, stream grpc.Stream) erro } if _, err := conn.Write(p.Data); err != nil { conn.Close() - return err + return errors.WithStack(err) } p.Data = p.Data[:0] } @@ -43,7 +44,7 @@ func Copy(ctx context.Context, conn io.ReadWriteCloser, stream grpc.Stream) erro case err == io.EOF: return nil case err != nil: - return err + return errors.WithStack(err) } select { case <-ctx.Done(): @@ -52,7 +53,7 @@ func Copy(ctx context.Context, conn io.ReadWriteCloser, stream grpc.Stream) erro } p := &BytesMessage{Data: buf[:n]} if err := stream.SendMsg(p); err != nil { - return err + return errors.WithStack(err) } } }) diff --git a/session/sshforward/ssh.go b/session/sshforward/ssh.go index a4effef60..660e89f7f 100644 --- a/session/sshforward/ssh.go +++ b/session/sshforward/ssh.go @@ -7,6 +7,7 @@ import ( "path/filepath" "github.com/moby/buildkit/session" + "github.com/pkg/errors" context "golang.org/x/net/context" "golang.org/x/sync/errgroup" "google.golang.org/grpc/metadata" @@ -65,7 +66,7 @@ type SocketOpt struct { func MountSSHSocket(ctx context.Context, c session.Caller, opt SocketOpt) (sockPath string, closer func() error, err error) { dir, err := ioutil.TempDir("", ".buildkit-ssh-sock") if err != nil { - return "", nil, err + return "", nil, errors.WithStack(err) } defer func() { @@ -78,16 +79,16 @@ func MountSSHSocket(ctx context.Context, c session.Caller, opt SocketOpt) (sockP l, err := net.Listen("unix", sockPath) if err != nil { - return "", nil, err + return "", nil, errors.WithStack(err) } if err := os.Chown(sockPath, opt.UID, opt.GID); err != nil { l.Close() - return "", nil, err + return "", nil, errors.WithStack(err) } if err := os.Chmod(sockPath, os.FileMode(opt.Mode)); err != nil { l.Close() - return "", nil, err + return "", nil, errors.WithStack(err) } s := &server{caller: c} @@ -102,12 +103,12 @@ func MountSSHSocket(ctx context.Context, c session.Caller, opt SocketOpt) (sockP return sockPath, func() error { err := l.Close() os.RemoveAll(sockPath) - return err + return errors.WithStack(err) }, nil } func CheckSSHID(ctx context.Context, c session.Caller, id string) error { client := NewSSHClient(c.Conn()) _, err := client.CheckAgent(ctx, &CheckAgentRequest{ID: id}) - return err + return errors.WithStack(err) } diff --git a/session/upload/upload.go b/session/upload/upload.go index 8d69bde25..c739b92d8 100644 --- a/session/upload/upload.go +++ b/session/upload/upload.go @@ -6,6 +6,7 @@ import ( "net/url" "github.com/moby/buildkit/session" + "github.com/pkg/errors" "google.golang.org/grpc/metadata" ) @@ -26,7 +27,7 @@ func New(ctx context.Context, c session.Caller, url *url.URL) (*Upload, error) { cc, err := client.Pull(ctx) if err != nil { - return nil, err + return nil, errors.WithStack(err) } return &Upload{cc: cc}, nil @@ -44,12 +45,12 @@ func (u *Upload) WriteTo(w io.Writer) (int, error) { if err == io.EOF { return n, nil } - return n, err + return n, errors.WithStack(err) } nn, err := w.Write(bm.Data) n += nn if err != nil { - return n, err + return n, errors.WithStack(err) } } } From b087d06adba7f349f09a7f7d3645ed33da2457ff Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sat, 1 Jun 2019 17:02:42 -0700 Subject: [PATCH 3/6] cache: error tracing on cache importer Signed-off-by: Tonis Tiigi --- cache/remotecache/import.go | 16 ++++++++-------- cache/remotecache/v1/cachestorage.go | 2 +- cache/remotecache/v1/parse.go | 2 +- cache/util/fsutil.go | 14 +++++++------- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cache/remotecache/import.go b/cache/remotecache/import.go index 6bbee9681..229d45a07 100644 --- a/cache/remotecache/import.go +++ b/cache/remotecache/import.go @@ -100,7 +100,7 @@ func readBlob(ctx context.Context, provider content.Provider, desc ocispec.Descr } } } - return dt, err + return dt, errors.WithStack(err) } func (ci *contentCacheImporter) importInlineCache(ctx context.Context, dt []byte, id string, w worker.Worker) (solver.CacheManager, error) { @@ -120,7 +120,7 @@ func (ci *contentCacheImporter) importInlineCache(ctx context.Context, dt []byte var m ocispec.Manifest if err := json.Unmarshal(dt, &m); err != nil { - return err + return errors.WithStack(err) } if m.Config.Digest == "" || len(m.Layers) == 0 { @@ -129,13 +129,13 @@ func (ci *contentCacheImporter) importInlineCache(ctx context.Context, dt []byte p, err := content.ReadBlob(ctx, ci.provider, m.Config) if err != nil { - return err + return errors.WithStack(err) } var img image if err := json.Unmarshal(p, &img); err != nil { - return err + return errors.WithStack(err) } if len(img.Rootfs.DiffIDs) != len(m.Layers) { @@ -149,7 +149,7 @@ func (ci *contentCacheImporter) importInlineCache(ctx context.Context, dt []byte var config v1.CacheConfig if err := json.Unmarshal(img.Cache, &config.Records); err != nil { - return err + return errors.WithStack(err) } createdDates, createdMsg, err := parseCreatedLayerInfo(img) @@ -181,7 +181,7 @@ func (ci *contentCacheImporter) importInlineCache(ctx context.Context, dt []byte dt, err = json.Marshal(config) if err != nil { - return err + return errors.WithStack(err) } mu.Lock() @@ -217,7 +217,7 @@ func (ci *contentCacheImporter) allDistributionManifests(ctx context.Context, dt case images.MediaTypeDockerSchema2ManifestList, ocispec.MediaTypeImageIndex: var index ocispec.Index if err := json.Unmarshal(dt, &index); err != nil { - return err + return errors.WithStack(err) } for _, d := range index.Manifests { @@ -226,7 +226,7 @@ func (ci *contentCacheImporter) allDistributionManifests(ctx context.Context, dt } p, err := content.ReadBlob(ctx, ci.provider, d) if err != nil { - return err + return errors.WithStack(err) } if err := ci.allDistributionManifests(ctx, p, m); err != nil { return err diff --git a/cache/remotecache/v1/cachestorage.go b/cache/remotecache/v1/cachestorage.go index 2061ffc07..fe5173935 100644 --- a/cache/remotecache/v1/cachestorage.go +++ b/cache/remotecache/v1/cachestorage.go @@ -254,7 +254,7 @@ func (cs *cacheResultStorage) Load(ctx context.Context, res solver.CacheResult) ref, err := cs.w.FromRemote(ctx, item.result) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "failed to load result from remote") } return worker.NewWorkerRefResult(ref, cs.w), nil } diff --git a/cache/remotecache/v1/parse.go b/cache/remotecache/v1/parse.go index 26b405019..79adf014a 100644 --- a/cache/remotecache/v1/parse.go +++ b/cache/remotecache/v1/parse.go @@ -12,7 +12,7 @@ import ( func Parse(configJSON []byte, provider DescriptorProvider, t solver.CacheExporterTarget) error { var config CacheConfig if err := json.Unmarshal(configJSON, &config); err != nil { - return err + return errors.WithStack(err) } return ParseConfig(config, provider, t) diff --git a/cache/util/fsutil.go b/cache/util/fsutil.go index b7aa6730d..41e5465f7 100644 --- a/cache/util/fsutil.go +++ b/cache/util/fsutil.go @@ -61,23 +61,23 @@ func ReadFile(ctx context.Context, ref cache.ImmutableRef, req ReadRequest) ([]b err := withMount(ctx, ref, func(root string) error { fp, err := fs.RootPath(root, req.Filename) if err != nil { - return err + return errors.WithStack(err) } if req.Range == nil { dt, err = ioutil.ReadFile(fp) if err != nil { - return err + return errors.WithStack(err) } } else { f, err := os.Open(fp) if err != nil { - return err + return errors.WithStack(err) } dt, err = ioutil.ReadAll(io.NewSectionReader(f, int64(req.Range.Offset), int64(req.Range.Length))) f.Close() if err != nil { - return err + return errors.WithStack(err) } } return nil @@ -101,7 +101,7 @@ func ReadDir(ctx context.Context, ref cache.ImmutableRef, req ReadDirRequest) ([ err := withMount(ctx, ref, func(root string) error { fp, err := fs.RootPath(root, req.Path) if err != nil { - return err + return errors.WithStack(err) } return fsutil.Walk(ctx, fp, &wo, func(path string, info os.FileInfo, err error) error { if err != nil { @@ -128,10 +128,10 @@ func StatFile(ctx context.Context, ref cache.ImmutableRef, path string) (*fstype err := withMount(ctx, ref, func(root string) error { fp, err := fs.RootPath(root, path) if err != nil { - return err + return errors.WithStack(err) } if st, err = fsutil.Stat(fp); err != nil { - return err + return errors.WithStack(err) } return nil }) From 61f1bc138babb77d8b7d684fecab5474fd7ef092 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sat, 1 Jun 2019 17:12:49 -0700 Subject: [PATCH 4/6] solver: add error tracing to edge connections Signed-off-by: Tonis Tiigi --- solver/edge.go | 10 ++++++---- solver/llbsolver/bridge.go | 6 +++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/solver/edge.go b/solver/edge.go index beee0a8dc..f1b57ec98 100644 --- a/solver/edge.go +++ b/solver/edge.go @@ -331,7 +331,8 @@ func (e *edge) unpark(incoming []pipe.Sender, updates, allPipes []pipe.Receiver, if e.cacheMapReq == nil && (e.cacheMap == nil || len(e.cacheRecords) == 0) { index := e.cacheMapIndex e.cacheMapReq = f.NewFuncRequest(func(ctx context.Context) (interface{}, error) { - return e.op.CacheMap(ctx, index) + cm, err := e.op.CacheMap(ctx, index) + return cm, errors.Wrapf(err, "failed to load cache key") }) cacheMapReq = true } @@ -798,7 +799,8 @@ func (e *edge) createInputRequests(desiredState edgeStatusType, f *pipeFactory, res := dep.result func(fn ResultBasedCacheFunc, res Result, index Index) { dep.slowCacheReq = f.NewFuncRequest(func(ctx context.Context) (interface{}, error) { - return e.op.CalcSlowCache(ctx, index, fn, res) + v, err := e.op.CalcSlowCache(ctx, index, fn, res) + return v, errors.Wrapf(err, "failed to compute cache key") }) }(fn, res, dep.index) addedNew = true @@ -850,7 +852,7 @@ func (e *edge) loadCache(ctx context.Context) (interface{}, error) { logrus.Debugf("load cache for %s with %s", e.edge.Vertex.Name(), rec.ID) res, err := e.op.LoadCache(ctx, rec) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "failed to load cache") } return NewCachedResult(res, []ExportableCacheKey{{CacheKey: rec.key, Exporter: &exporter{k: rec.key, record: rec, edge: e}}}), nil @@ -861,7 +863,7 @@ func (e *edge) execOp(ctx context.Context) (interface{}, error) { cacheKeys, inputs := e.commitOptions() results, subExporters, err := e.op.Exec(ctx, toResultSlice(inputs)) if err != nil { - return nil, err + return nil, errors.WithStack(err) } index := e.edge.Index diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go index 137c8acf5..c55ff167c 100644 --- a/solver/llbsolver/bridge.go +++ b/solver/llbsolver/bridge.go @@ -94,11 +94,11 @@ func (b *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest) (res * edge, err := Load(req.Definition, ValidateEntitlements(ent), WithCacheSources(cms), RuntimePlatforms(b.platforms), WithValidateCaps()) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "failed to load LLB") } ref, err := b.builder.Build(ctx, edge) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "failed to build LLB") } res = &frontend.Result{Ref: ref} @@ -109,7 +109,7 @@ func (b *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest) (res * } res, err = f.Solve(ctx, b, req.FrontendOpt) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "failed to solve with frontend %s", req.Frontend) } } else { return &frontend.Result{}, nil From 0f1c7d0412e3e21f6ca68ebfa5e6756499291237 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sat, 1 Jun 2019 17:55:52 -0700 Subject: [PATCH 5/6] session: use errors cause Signed-off-by: Tonis Tiigi --- frontend/gateway/grpcclient/client.go | 2 +- session/auth/auth.go | 2 +- session/secrets/secrets.go | 2 +- solver/llbsolver/ops/exec.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/gateway/grpcclient/client.go b/frontend/gateway/grpcclient/client.go index b39b28081..1a1ff0757 100644 --- a/frontend/gateway/grpcclient/client.go +++ b/frontend/gateway/grpcclient/client.go @@ -128,7 +128,7 @@ func (c *grpcClient) Run(ctx context.Context, f client.BuildFunc) (retError erro } } if retError != nil { - st, _ := status.FromError(retError) + st, _ := status.FromError(errors.Cause(retError)) stp := st.Proto() req.Error = &rpc.Status{ Code: stp.Code, diff --git a/session/auth/auth.go b/session/auth/auth.go index f2cfa6699..5717455f8 100644 --- a/session/auth/auth.go +++ b/session/auth/auth.go @@ -17,7 +17,7 @@ func CredentialsFunc(ctx context.Context, c session.Caller) func(string) (string Host: host, }) if err != nil { - if st, ok := status.FromError(err); ok && st.Code() == codes.Unimplemented { + if st, ok := status.FromError(errors.Cause(err)); ok && st.Code() == codes.Unimplemented { return "", "", nil } return "", "", errors.WithStack(err) diff --git a/session/secrets/secrets.go b/session/secrets/secrets.go index 1d27430d9..3f3bb6448 100644 --- a/session/secrets/secrets.go +++ b/session/secrets/secrets.go @@ -21,7 +21,7 @@ func GetSecret(ctx context.Context, c session.Caller, id string) ([]byte, error) ID: id, }) if err != nil { - if st, ok := status.FromError(err); ok && (st.Code() == codes.Unimplemented || st.Code() == codes.NotFound) { + if st, ok := status.FromError(errors.Cause(err)); ok && (st.Code() == codes.Unimplemented || st.Code() == codes.NotFound) { return nil, errors.Wrapf(ErrNotFound, "secret %s not found", id) } return nil, errors.WithStack(err) diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 00f0f128d..d1a71c432 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -324,7 +324,7 @@ func (e *execOp) getSSHMountable(ctx context.Context, m *pb.Mount) (cache.Mounta if m.SSHOpt.Optional { return nil, nil } - if st, ok := status.FromError(err); ok && st.Code() == codes.Unimplemented { + if st, ok := status.FromError(errors.Cause(err)); ok && st.Code() == codes.Unimplemented { return nil, errors.Errorf("no SSH key %q forwarded from the client", m.SSHOpt.ID) } return nil, err From e2dcafa5ca8aa33cd2caa82190a44c04dd8932c5 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Fri, 7 Jun 2019 12:04:35 -0700 Subject: [PATCH 6/6] Removing wrapf for review Signed-off-by: Tonis Tiigi --- cache/remotecache/v1/cachestorage.go | 2 +- solver/edge.go | 6 +++--- solver/llbsolver/bridge.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cache/remotecache/v1/cachestorage.go b/cache/remotecache/v1/cachestorage.go index fe5173935..605b6d634 100644 --- a/cache/remotecache/v1/cachestorage.go +++ b/cache/remotecache/v1/cachestorage.go @@ -254,7 +254,7 @@ func (cs *cacheResultStorage) Load(ctx context.Context, res solver.CacheResult) ref, err := cs.w.FromRemote(ctx, item.result) if err != nil { - return nil, errors.Wrapf(err, "failed to load result from remote") + return nil, errors.Wrap(err, "failed to load result from remote") } return worker.NewWorkerRefResult(ref, cs.w), nil } diff --git a/solver/edge.go b/solver/edge.go index f1b57ec98..b809652c4 100644 --- a/solver/edge.go +++ b/solver/edge.go @@ -332,7 +332,7 @@ func (e *edge) unpark(incoming []pipe.Sender, updates, allPipes []pipe.Receiver, index := e.cacheMapIndex e.cacheMapReq = f.NewFuncRequest(func(ctx context.Context) (interface{}, error) { cm, err := e.op.CacheMap(ctx, index) - return cm, errors.Wrapf(err, "failed to load cache key") + return cm, errors.Wrap(err, "failed to load cache key") }) cacheMapReq = true } @@ -800,7 +800,7 @@ func (e *edge) createInputRequests(desiredState edgeStatusType, f *pipeFactory, func(fn ResultBasedCacheFunc, res Result, index Index) { dep.slowCacheReq = f.NewFuncRequest(func(ctx context.Context) (interface{}, error) { v, err := e.op.CalcSlowCache(ctx, index, fn, res) - return v, errors.Wrapf(err, "failed to compute cache key") + return v, errors.Wrap(err, "failed to compute cache key") }) }(fn, res, dep.index) addedNew = true @@ -852,7 +852,7 @@ func (e *edge) loadCache(ctx context.Context) (interface{}, error) { logrus.Debugf("load cache for %s with %s", e.edge.Vertex.Name(), rec.ID) res, err := e.op.LoadCache(ctx, rec) if err != nil { - return nil, errors.Wrapf(err, "failed to load cache") + return nil, errors.Wrap(err, "failed to load cache") } return NewCachedResult(res, []ExportableCacheKey{{CacheKey: rec.key, Exporter: &exporter{k: rec.key, record: rec, edge: e}}}), nil diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go index c55ff167c..e5d362d80 100644 --- a/solver/llbsolver/bridge.go +++ b/solver/llbsolver/bridge.go @@ -94,11 +94,11 @@ func (b *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest) (res * edge, err := Load(req.Definition, ValidateEntitlements(ent), WithCacheSources(cms), RuntimePlatforms(b.platforms), WithValidateCaps()) if err != nil { - return nil, errors.Wrapf(err, "failed to load LLB") + return nil, errors.Wrap(err, "failed to load LLB") } ref, err := b.builder.Build(ctx, edge) if err != nil { - return nil, errors.Wrapf(err, "failed to build LLB") + return nil, errors.Wrap(err, "failed to build LLB") } res = &frontend.Result{Ref: ref}