Handle context cancellation during builds (#539)

`buildx.Build` doesn't terminate if context is canceled, so this PR
rearranges things such that we can wait for the build or context to
finish.

Fixes #533.
This commit is contained in:
Bryce Lampe
2025-05-05 16:12:48 -07:00
committed by GitHub
parent 1cba556bd2
commit 30a01b6893
8 changed files with 197 additions and 257 deletions

View File

@@ -57,11 +57,11 @@ type cli struct {
auths map[string]cfgtypes.AuthConfig
host *host
in string // stdin
r, w *os.File // stdout
err bytes.Buffer // stderr
dumplogs bool // if true then tail() will re-log status messages
done chan struct{} // signaled when all logs have been forwarded to the engine.
in string // stdin
r, w *os.File // stdout
err bytes.Buffer // stderr
dumplogs bool // if true then tail() will re-log status messages
builder Builder // for mocking build daemon responses
}
// Cli wraps the Docker interface for mock generation.
@@ -120,11 +120,12 @@ func wrap(host *host, registries ...Registry) (*cli, error) {
}
wrapped := &cli{
Cli: docker,
host: host,
auths: auths,
r: r,
w: w,
Cli: docker,
host: host,
auths: auths,
r: r,
w: w,
builder: defaultBuilder{},
}
return wrapped, nil
@@ -163,14 +164,6 @@ func (c *cli) rc() *regclient.RegClient {
// tail is meant to be called as a goroutine and will pipe output from the CLI
// back to the Pulumi engine. Requires a corresponding call to close.
func (c *cli) tail(ctx context.Context) {
c.done = make(chan struct{}, 1)
defer func() {
c.done <- struct{}{}
if err := recover(); err != nil {
fmt.Fprintf(os.Stderr, "recovered: %s\n", err)
}
}()
b := bytes.Buffer{}
s := bufio.NewScanner(c.r)
@@ -194,11 +187,7 @@ func (c *cli) tail(ctx context.Context) {
// close flushes any outstanding logs and cleans up resources.
func (c *cli) Close() error {
err := errors.Join(c.w.Close(), c.r.Close())
if c.done != nil {
<-c.done
}
return err
return errors.Join(c.w.Close(), c.r.Close())
}
// execBuild performs a build by os.Exec'ing the docker-buildx binary.

View File

@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//go:generate go run go.uber.org/mock/mockgen -typed -package internal -source client.go -destination mockclient_test.go --self_package github.com/pulumi/pulumi-docker-build/provider/internal
//go:generate go run go.uber.org/mock/mockgen -typed -package internal -source client.go -destination mockclient_test.go --self_package github.com/pulumi/pulumi-docker-build/provider/internal -imports buildx=github.com/docker/buildx/build
package internal
@@ -26,6 +26,7 @@ import (
"github.com/distribution/reference"
buildx "github.com/docker/buildx/build"
"github.com/docker/buildx/builder"
"github.com/docker/buildx/commands"
controllerapi "github.com/docker/buildx/controller/pb"
"github.com/docker/buildx/util/confutil"
@@ -123,9 +124,15 @@ func (c *cli) Build(
if err != nil {
return nil, fmt.Errorf("creating printer: %w", err)
}
defer func() {
// Log any warnings when we're done.
_ = printer.Wait()
// Wait for logs to flush if the build finished, but not if we're
// exiting early.
if ctx.Err() == nil {
_ = printer.Wait()
}
// Log any warnings we got, separated by newlines.
for _, w := range printer.Warnings() {
b := &bytes.Buffer{}
_, _ = b.Write(w.Short)
@@ -224,21 +231,40 @@ func (c *cli) Build(
},
}
// Perform the build.
results, err := buildx.Build(
ctx,
b.nodes,
payload,
dockerutil.NewClient(c),
confutil.NewConfig(c),
printer,
)
if err != nil {
resultC := make(chan map[string]*client.SolveResponse)
errC := make(chan error)
// buildx.Build doesn't handle context cancellation, so we monitor it in a
// goroutine. cli.Close cleans up our file descriptors, so if we do exit
// early the remote build should terminate as soon as it sees the pipe has
// broken.
go func() {
defer close(resultC)
defer close(errC)
results, err := c.builder.Build(
ctx,
b.nodes,
payload,
dockerutil.NewClient(c),
confutil.NewConfig(c),
printer,
)
if err != nil {
errC <- err
return
}
resultC <- results
}()
select {
case results := <-resultC:
return results[target], nil
case err := <-errC:
c.dumplogs = true
return nil, err
case <-ctx.Done():
return nil, ctx.Err()
}
return results[target], err
}
// BuildKitEnabled returns true if the client supports buildkit.
@@ -354,6 +380,31 @@ func (c *cli) Delete(ctx context.Context, r string) error {
return nil
}
// Builder allows injecting mock responses from the build daemon.
type Builder interface {
Build(
ctx context.Context,
nodes []builder.Node,
opts map[string]buildx.Options,
docker *dockerutil.Client,
cfg *confutil.Config,
w progress.Writer,
) (resp map[string]*client.SolveResponse, err error)
}
type defaultBuilder struct{}
func (defaultBuilder) Build(
ctx context.Context,
nodes []builder.Node,
opts map[string]buildx.Options,
docker *dockerutil.Client,
cfg *confutil.Config,
w progress.Writer,
) (resp map[string]*client.SolveResponse, err error) {
return buildx.Build(ctx, nodes, opts, docker, cfg, w)
}
func normalizeReference(ref string) (reference.Named, error) {
namedRef, err := reference.ParseNormalizedNamed(ref)
if err != nil {

View File

@@ -17,15 +17,23 @@ package internal
import (
"bytes"
"context"
"errors"
"io"
"log/slog"
"os"
"path/filepath"
"testing"
buildx "github.com/docker/buildx/build"
"github.com/docker/buildx/builder"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/dockerutil"
"github.com/docker/buildx/util/progress"
"github.com/docker/docker/api/types/registry"
"github.com/moby/buildkit/client"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
)
func TestAuth(t *testing.T) {
@@ -432,6 +440,35 @@ func TestBuildExecError(t *testing.T) {
}
}
func TestBuildCancelation(t *testing.T) {
t.Parallel()
cli := testcli(t, true)
ctrl := gomock.NewController(t)
ctx, cancel := context.WithCancel(context.Background())
b := NewMockBuilder(ctrl)
b.EXPECT().Build(
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
).DoAndReturn(func(
_ context.Context,
_ []builder.Node,
_ map[string]buildx.Options,
_ *dockerutil.Client,
_ *confutil.Config,
_ progress.Writer,
) (map[string]*client.SolveResponse, error) {
cancel()
return nil, errors.New("cancel wasn't respected")
})
cli.builder = b
resp, err := cli.Build(ctx, &build{})
assert.ErrorIs(t, err, context.Canceled)
assert.Nil(t, resp)
}
// testcli returns a new standalone CLI instance. Set ping to true if a live
// daemon is required -- the test will be skipped if the daemon is not available.
func testcli(t *testing.T, ping bool, auths ...Registry) *cli {

View File

@@ -16,12 +16,8 @@ import (
configfile "github.com/docker/cli/cli/config/configfile"
docker "github.com/docker/cli/cli/context/docker"
store "github.com/docker/cli/cli/context/store"
store0 "github.com/docker/cli/cli/manifest/store"
client "github.com/docker/cli/cli/registry/client"
streams "github.com/docker/cli/cli/streams"
trust "github.com/docker/cli/cli/trust"
client0 "github.com/docker/docker/client"
client1 "github.com/theupdateframework/notary/client"
client "github.com/docker/docker/client"
metric "go.opentelemetry.io/otel/metric"
resource "go.opentelemetry.io/otel/sdk/resource"
trace "go.opentelemetry.io/otel/trace"
@@ -134,10 +130,10 @@ func (c *MockCliBuildKitEnabledCall) DoAndReturn(f func() (bool, error)) *MockCl
}
// Client mocks base method.
func (m *MockCli) Client() client0.APIClient {
func (m *MockCli) Client() client.APIClient {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Client")
ret0, _ := ret[0].(client0.APIClient)
ret0, _ := ret[0].(client.APIClient)
return ret0
}
@@ -154,19 +150,19 @@ type MockCliClientCall struct {
}
// Return rewrite *gomock.Call.Return
func (c *MockCliClientCall) Return(arg0 client0.APIClient) *MockCliClientCall {
func (c *MockCliClientCall) Return(arg0 client.APIClient) *MockCliClientCall {
c.Call = c.Call.Return(arg0)
return c
}
// Do rewrite *gomock.Call.Do
func (c *MockCliClientCall) Do(f func() client0.APIClient) *MockCliClientCall {
func (c *MockCliClientCall) Do(f func() client.APIClient) *MockCliClientCall {
c.Call = c.Call.Do(f)
return c
}
// DoAndReturn rewrite *gomock.Call.DoAndReturn
func (c *MockCliClientCall) DoAndReturn(f func() client0.APIClient) *MockCliClientCall {
func (c *MockCliClientCall) DoAndReturn(f func() client.APIClient) *MockCliClientCall {
c.Call = c.Call.DoAndReturn(f)
return c
}
@@ -513,44 +509,6 @@ func (c *MockCliInCall) DoAndReturn(f func() *streams.In) *MockCliInCall {
return c
}
// ManifestStore mocks base method.
func (m *MockCli) ManifestStore() store0.Store {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ManifestStore")
ret0, _ := ret[0].(store0.Store)
return ret0
}
// ManifestStore indicates an expected call of ManifestStore.
func (mr *MockCliMockRecorder) ManifestStore() *MockCliManifestStoreCall {
mr.mock.ctrl.T.Helper()
call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ManifestStore", reflect.TypeOf((*MockCli)(nil).ManifestStore))
return &MockCliManifestStoreCall{Call: call}
}
// MockCliManifestStoreCall wrap *gomock.Call
type MockCliManifestStoreCall struct {
*gomock.Call
}
// Return rewrite *gomock.Call.Return
func (c *MockCliManifestStoreCall) Return(arg0 store0.Store) *MockCliManifestStoreCall {
c.Call = c.Call.Return(arg0)
return c
}
// Do rewrite *gomock.Call.Do
func (c *MockCliManifestStoreCall) Do(f func() store0.Store) *MockCliManifestStoreCall {
c.Call = c.Call.Do(f)
return c
}
// DoAndReturn rewrite *gomock.Call.DoAndReturn
func (c *MockCliManifestStoreCall) DoAndReturn(f func() store0.Store) *MockCliManifestStoreCall {
c.Call = c.Call.DoAndReturn(f)
return c
}
// MeterProvider mocks base method.
func (m *MockCli) MeterProvider() metric.MeterProvider {
m.ctrl.T.Helper()
@@ -589,45 +547,6 @@ func (c *MockCliMeterProviderCall) DoAndReturn(f func() metric.MeterProvider) *M
return c
}
// NotaryClient mocks base method.
func (m *MockCli) NotaryClient(imgRefAndAuth trust.ImageRefAndAuth, actions []string) (client1.Repository, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "NotaryClient", imgRefAndAuth, actions)
ret0, _ := ret[0].(client1.Repository)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// NotaryClient indicates an expected call of NotaryClient.
func (mr *MockCliMockRecorder) NotaryClient(imgRefAndAuth, actions any) *MockCliNotaryClientCall {
mr.mock.ctrl.T.Helper()
call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NotaryClient", reflect.TypeOf((*MockCli)(nil).NotaryClient), imgRefAndAuth, actions)
return &MockCliNotaryClientCall{Call: call}
}
// MockCliNotaryClientCall wrap *gomock.Call
type MockCliNotaryClientCall struct {
*gomock.Call
}
// Return rewrite *gomock.Call.Return
func (c *MockCliNotaryClientCall) Return(arg0 client1.Repository, arg1 error) *MockCliNotaryClientCall {
c.Call = c.Call.Return(arg0, arg1)
return c
}
// Do rewrite *gomock.Call.Do
func (c *MockCliNotaryClientCall) Do(f func(trust.ImageRefAndAuth, []string) (client1.Repository, error)) *MockCliNotaryClientCall {
c.Call = c.Call.Do(f)
return c
}
// DoAndReturn rewrite *gomock.Call.DoAndReturn
func (c *MockCliNotaryClientCall) DoAndReturn(f func(trust.ImageRefAndAuth, []string) (client1.Repository, error)) *MockCliNotaryClientCall {
c.Call = c.Call.DoAndReturn(f)
return c
}
// Out mocks base method.
func (m *MockCli) Out() *streams.Out {
m.ctrl.T.Helper()
@@ -666,44 +585,6 @@ func (c *MockCliOutCall) DoAndReturn(f func() *streams.Out) *MockCliOutCall {
return c
}
// RegistryClient mocks base method.
func (m *MockCli) RegistryClient(arg0 bool) client.RegistryClient {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "RegistryClient", arg0)
ret0, _ := ret[0].(client.RegistryClient)
return ret0
}
// RegistryClient indicates an expected call of RegistryClient.
func (mr *MockCliMockRecorder) RegistryClient(arg0 any) *MockCliRegistryClientCall {
mr.mock.ctrl.T.Helper()
call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegistryClient", reflect.TypeOf((*MockCli)(nil).RegistryClient), arg0)
return &MockCliRegistryClientCall{Call: call}
}
// MockCliRegistryClientCall wrap *gomock.Call
type MockCliRegistryClientCall struct {
*gomock.Call
}
// Return rewrite *gomock.Call.Return
func (c *MockCliRegistryClientCall) Return(arg0 client.RegistryClient) *MockCliRegistryClientCall {
c.Call = c.Call.Return(arg0)
return c
}
// Do rewrite *gomock.Call.Do
func (c *MockCliRegistryClientCall) Do(f func(bool) client.RegistryClient) *MockCliRegistryClientCall {
c.Call = c.Call.Do(f)
return c
}
// DoAndReturn rewrite *gomock.Call.DoAndReturn
func (c *MockCliRegistryClientCall) DoAndReturn(f func(bool) client.RegistryClient) *MockCliRegistryClientCall {
c.Call = c.Call.DoAndReturn(f)
return c
}
// Resource mocks base method.
func (m *MockCli) Resource() *resource.Resource {
m.ctrl.T.Helper()

View File

@@ -3,7 +3,7 @@
//
// Generated by this command:
//
// mockgen -typed -package internal -source client.go -destination mockclient_test.go --self_package github.com/pulumi/pulumi-docker-build/provider/internal
// mockgen -typed -package internal -source client.go -destination mockclient_test.go --self_package github.com/pulumi/pulumi-docker-build/provider/internal -imports buildx=github.com/docker/buildx/build
//
// Package internal is a generated GoMock package.
@@ -13,7 +13,12 @@ import (
context "context"
reflect "reflect"
buildx "github.com/docker/buildx/build"
builder "github.com/docker/buildx/builder"
pb "github.com/docker/buildx/controller/pb"
confutil "github.com/docker/buildx/util/confutil"
dockerutil "github.com/docker/buildx/util/dockerutil"
progress "github.com/docker/buildx/util/progress"
client "github.com/moby/buildkit/client"
session "github.com/moby/buildkit/session"
descriptor "github.com/regclient/regclient/types/descriptor"
@@ -532,3 +537,66 @@ func (c *MockBuildShouldExecCall) DoAndReturn(f func() bool) *MockBuildShouldExe
c.Call = c.Call.DoAndReturn(f)
return c
}
// MockBuilder is a mock of Builder interface.
type MockBuilder struct {
ctrl *gomock.Controller
recorder *MockBuilderMockRecorder
isgomock struct{}
}
// MockBuilderMockRecorder is the mock recorder for MockBuilder.
type MockBuilderMockRecorder struct {
mock *MockBuilder
}
// NewMockBuilder creates a new mock instance.
func NewMockBuilder(ctrl *gomock.Controller) *MockBuilder {
mock := &MockBuilder{ctrl: ctrl}
mock.recorder = &MockBuilderMockRecorder{mock}
return mock
}
// EXPECT returns an object that allows the caller to indicate expected use.
func (m *MockBuilder) EXPECT() *MockBuilderMockRecorder {
return m.recorder
}
// Build mocks base method.
func (m *MockBuilder) Build(ctx context.Context, nodes []builder.Node, opts map[string]buildx.Options, docker *dockerutil.Client, cfg *confutil.Config, w progress.Writer) (map[string]*client.SolveResponse, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Build", ctx, nodes, opts, docker, cfg, w)
ret0, _ := ret[0].(map[string]*client.SolveResponse)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// Build indicates an expected call of Build.
func (mr *MockBuilderMockRecorder) Build(ctx, nodes, opts, docker, cfg, w any) *MockBuilderBuildCall {
mr.mock.ctrl.T.Helper()
call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Build", reflect.TypeOf((*MockBuilder)(nil).Build), ctx, nodes, opts, docker, cfg, w)
return &MockBuilderBuildCall{Call: call}
}
// MockBuilderBuildCall wrap *gomock.Call
type MockBuilderBuildCall struct {
*gomock.Call
}
// Return rewrite *gomock.Call.Return
func (c *MockBuilderBuildCall) Return(resp map[string]*client.SolveResponse, err error) *MockBuilderBuildCall {
c.Call = c.Call.Return(resp, err)
return c
}
// Do rewrite *gomock.Call.Do
func (c *MockBuilderBuildCall) Do(f func(context.Context, []builder.Node, map[string]buildx.Options, *dockerutil.Client, *confutil.Config, progress.Writer) (map[string]*client.SolveResponse, error)) *MockBuilderBuildCall {
c.Call = c.Call.Do(f)
return c
}
// DoAndReturn rewrite *gomock.Call.DoAndReturn
func (c *MockBuilderBuildCall) DoAndReturn(f func(context.Context, []builder.Node, map[string]buildx.Options, *dockerutil.Client, *confutil.Config, progress.Writer) (map[string]*client.SolveResponse, error)) *MockBuilderBuildCall {
c.Call = c.Call.DoAndReturn(f)
return c
}