Return committed readonly inputs and actives in exec error in MountIDs

Signed-off-by: Edgar Lee <edgarl@netflix.com>
v0.8
Edgar Lee 2020-11-16 13:20:45 -08:00
parent 3ba6cd7bc2
commit 1240dd7795
5 changed files with 63 additions and 37 deletions

View File

@ -1118,6 +1118,17 @@ func testClientGatewayExecError(t *testing.T, sb integration.Sandbox) {
llb.AddMount("/readonly", llb.Scratch(), llb.Readonly),
).Root(),
2, []string{"/data"},
}, {
"rootfs and readwrite force no output mount",
llb.Image("busybox:latest").Run(
llb.Shlexf(`sh -c "echo %s > /data && echo %s > /rw/data && fail"`, id, id),
llb.AddMount(
"/rw",
llb.Scratch().File(llb.Mkfile("foo", 0700, []byte(id))),
llb.ForceNoOutput,
),
).Root(),
2, []string{"/data", "/rw/data", "/rw/foo"},
}}
for _, tt := range tests {
@ -1216,7 +1227,7 @@ func testClientGatewayExecError(t *testing.T, sb integration.Sandbox) {
_, err = c.Build(ctx, SolveOpt{}, "buildkit_test", b, nil)
require.NoError(t, err)
// checkAllReleasable(t, c, sb, true)
checkAllReleasable(t, c, sb, true)
}
// testClientGatewaySlowCacheExecError is testing gateway exec into the ref

View File

@ -85,7 +85,7 @@ func NewContainer(ctx context.Context, w worker.Worker, sm *session.Manager, g s
})
if err != nil {
for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order
p.Actives[i].Release(context.TODO())
p.Actives[i].Ref.Release(context.TODO())
}
for _, o := range p.OutputRefs {
o.Ref.Release(context.TODO())
@ -104,7 +104,7 @@ func NewContainer(ctx context.Context, w worker.Worker, sm *session.Manager, g s
for _, active := range p.Actives {
active := active
ctr.cleanup = append(ctr.cleanup, func() error {
return active.Release(context.TODO())
return active.Ref.Release(context.TODO())
})
}
@ -115,15 +115,20 @@ type PreparedMounts struct {
Root executor.Mount
ReadonlyRootFS bool
Mounts []executor.Mount
OutputRefs []OutputRef
Actives []cache.MutableRef
OutputRefs []MountRef
Actives []MountMutableRef
}
type OutputRef struct {
type MountRef struct {
Ref cache.Ref
MountIndex int
}
type MountMutableRef struct {
Ref cache.MutableRef
MountIndex int
}
type MakeMutable func(m *opspb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error)
func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manager, g session.Group, mnts []*opspb.Mount, refs []*worker.WorkerRef, makeMutable MakeMutable) (p PreparedMounts, err error) {
@ -140,7 +145,7 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage
// if mount is based on input validate and load it
if m.Input != opspb.Empty {
if int(m.Input) > len(refs) {
if int(m.Input) >= len(refs) {
return p, errors.Errorf("missing input %d", m.Input)
}
ref = refs[int(m.Input)].ImmutableRef
@ -153,7 +158,7 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage
if m.Output != opspb.SkipOutput {
// if it is readonly and not root then output is the input
if m.Readonly && ref != nil && m.Dest != opspb.RootMount {
p.OutputRefs = append(p.OutputRefs, OutputRef{
p.OutputRefs = append(p.OutputRefs, MountRef{
MountIndex: i,
Ref: ref.Clone(),
})
@ -164,7 +169,7 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage
return p, err
}
mountable = active
p.OutputRefs = append(p.OutputRefs, OutputRef{
p.OutputRefs = append(p.OutputRefs, MountRef{
MountIndex: i,
Ref: active,
})
@ -175,7 +180,10 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage
if err != nil {
return p, err
}
p.Actives = append(p.Actives, active)
p.Actives = append(p.Actives, MountMutableRef{
MountIndex: i,
Ref: active,
})
mountable = active
}
@ -185,9 +193,12 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage
return p, err
}
mountable = active
p.Actives = append(p.Actives, active)
p.Actives = append(p.Actives, MountMutableRef{
MountIndex: i,
Ref: active,
})
if m.Output != opspb.SkipOutput && ref != nil {
p.OutputRefs = append(p.OutputRefs, OutputRef{
p.OutputRefs = append(p.OutputRefs, MountRef{
MountIndex: i,
Ref: ref.Clone(),
})
@ -232,7 +243,10 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage
if err != nil {
return p, err
}
p.Actives = append(p.Actives, active)
p.Actives = append(p.Actives, MountMutableRef{
MountIndex: i,
Ref: active,
})
root = active
}
p.Root = mountWithSession(root, g)

View File

@ -10,6 +10,7 @@ import (
"github.com/moby/buildkit/client"
"github.com/moby/buildkit/session"
"github.com/moby/buildkit/solver/errdefs"
"github.com/moby/buildkit/solver/pb"
"github.com/moby/buildkit/util/flightcontrol"
"github.com/moby/buildkit/util/progress"
"github.com/moby/buildkit/util/tracing"
@ -621,7 +622,9 @@ func (s *sharedOp) LoadCache(ctx context.Context, rec *CacheRecord) (Result, err
// evaluated, hence "slow" cache.
func (s *sharedOp) CalcSlowCache(ctx context.Context, index Index, p PreprocessFunc, f ResultBasedCacheFunc, res Result) (dgst digest.Digest, err error) {
defer func() {
err = errdefs.WrapVertex(WrapSlowCache(err, index, NewSharedResult(res).Clone()), s.st.origDigest)
err = WrapSlowCache(err, index, NewSharedResult(res).Clone())
err = errdefs.WithOp(err, s.st.vtx.Sys().(*pb.Op))
err = errdefs.WrapVertex(err, s.st.origDigest)
}()
key, err := s.g.Do(ctx, fmt.Sprintf("slow-compute-%d", index), func(ctx context.Context) (interface{}, error) {
s.slowMu.Lock()
@ -683,6 +686,7 @@ func (s *sharedOp) CalcSlowCache(ctx context.Context, index Index, p PreprocessF
func (s *sharedOp) CacheMap(ctx context.Context, index int) (resp *cacheMapResp, err error) {
defer func() {
err = errdefs.WithOp(err, s.st.vtx.Sys().(*pb.Op))
err = errdefs.WrapVertex(err, s.st.origDigest)
}()
op, err := s.getOp()
@ -741,6 +745,7 @@ func (s *sharedOp) CacheMap(ctx context.Context, index int) (resp *cacheMapResp,
func (s *sharedOp) Exec(ctx context.Context, inputs []Result) (outputs []Result, exporters []ExportableCacheKey, err error) {
defer func() {
err = errdefs.WithOp(err, s.st.vtx.Sys().(*pb.Op))
err = errdefs.WrapVertex(err, s.st.origDigest)
}()
op, err := s.getOp()

View File

@ -198,21 +198,6 @@ func (rp *resultProxy) wrapError(err error) error {
}
}
}
var op *pb.Op
for _, dt := range rp.def.Def {
var curr pb.Op
if err := (&curr).Unmarshal(dt); err != nil {
return errors.Wrap(err, "failed to parse llb proto op")
}
if ve.Digest == digest.FromBytes(dt).String() {
op = &curr
break
}
}
if op != nil {
err = errdefs.WithOp(err, op)
}
}
return err
}

View File

@ -229,14 +229,6 @@ func (e *execOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu
return e.cm.New(ctx, ref, g, cache.WithDescription(desc))
})
defer func() {
for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order
p.Actives[i].Release(context.TODO())
}
for _, o := range p.OutputRefs {
if o.Ref != nil {
o.Ref.Release(context.TODO())
}
}
if err != nil {
execInputs := make([]solver.Result, len(e.op.Mounts))
for i, m := range e.op.Mounts {
@ -246,10 +238,29 @@ func (e *execOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu
execInputs[i] = inputs[m.Input]
}
execMounts := make([]solver.Result, len(e.op.Mounts))
copy(execMounts, execInputs)
for i, res := range results {
execMounts[p.OutputRefs[i].MountIndex] = res
}
for _, active := range p.Actives {
ref, cerr := active.Ref.Commit(ctx)
if cerr != nil {
err = errors.Wrapf(err, "error committing %s: %s", active.Ref.ID(), cerr)
continue
}
execMounts[active.MountIndex] = worker.NewWorkerRefResult(ref, e.w)
}
err = errdefs.WithExecError(err, execInputs, execMounts)
} else {
// Only release actives if err is nil.
for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order
p.Actives[i].Ref.Release(context.TODO())
}
}
for _, o := range p.OutputRefs {
if o.Ref != nil {
o.Ref.Release(context.TODO())
}
}
}()
if err != nil {