Skip to content

Commit

Permalink
Merge pull request #3233 from sipsma/fix-nil-release
Browse files Browse the repository at this point in the history
Fix nil panic on scratch ref release.
  • Loading branch information
tonistiigi authored Oct 30, 2022
2 parents 9940833 + 0ebdec8 commit b9a93f4
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 1 deletion.
49 changes: 49 additions & 0 deletions client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func TestClientGatewayIntegration(t *testing.T) {
testWarnings,
testClientGatewayFrontendAttrs,
testClientGatewayNilResult,
testClientGatewayEmptyImageExec,
), integration.WithMirroredImages(integration.OfficialImages("busybox:latest")))

integration.Run(t, integration.TestFuncs(
Expand Down Expand Up @@ -2086,6 +2087,54 @@ func testClientGatewayNilResult(t *testing.T, sb integration.Sandbox) {
require.NoError(t, err)
}

func testClientGatewayEmptyImageExec(t *testing.T, sb integration.Sandbox) {
integration.SkipIfDockerd(t, sb, "direct push")
c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

registry, err := sb.NewRegistry()
if errors.Is(err, integration.ErrRequirements) {
t.Skip(err.Error())
}
require.NoError(t, err)
target := registry + "/buildkit/testemptyimage:latest"

// push an empty image
_, err = c.Build(sb.Context(), SolveOpt{
Exports: []ExportEntry{
{
Type: ExporterImage,
Attrs: map[string]string{
"name": target,
"push": "true",
},
},
},
}, "", func(ctx context.Context, c client.Client) (*client.Result, error) {
return client.NewResult(), nil
}, nil)
require.NoError(t, err)

_, err = c.Build(sb.Context(), SolveOpt{}, "", func(ctx context.Context, gw client.Client) (*client.Result, error) {
// create an exec on that empty image (expected to fail, but not to panic)
st := llb.Image(target).Run(
llb.Args([]string{"echo", "hello"}),
).Root()
def, err := st.Marshal(sb.Context())
if err != nil {
return nil, err
}
_, err = gw.Solve(ctx, client.SolveRequest{
Definition: def.ToPB(),
Evaluate: true,
})
require.ErrorContains(t, err, `process "echo hello" did not complete successfully`)
return nil, nil
}, nil)
require.NoError(t, err)
}

type nopCloser struct {
io.Writer
}
Expand Down
2 changes: 1 addition & 1 deletion frontend/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ func (lbf *llbBridgeForwarder) Discard() {
}

for id, workerRef := range lbf.workerRefByID {
workerRef.ImmutableRef.Release(context.TODO())
workerRef.Release(context.TODO())
delete(lbf.workerRefByID, id)
}
if lbf.err != nil && lbf.result != nil {
Expand Down
7 changes: 7 additions & 0 deletions worker/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ func (wr *WorkerRef) ID() string {
return wr.Worker.ID() + "::" + refID
}

func (wr *WorkerRef) Release(ctx context.Context) error {
if wr.ImmutableRef == nil {
return nil
}
return wr.ImmutableRef.Release(ctx)
}

// GetRemotes method abstracts ImmutableRef's GetRemotes to allow a Worker to override.
// This is needed for moby integration.
// Use this method instead of calling ImmutableRef.GetRemotes() directly.
Expand Down

0 comments on commit b9a93f4

Please sign in to comment.