Flow context.Context through image builds (#522)

This should help with cancel responsiveness. Right now, there are
multiple places where docker does not respond to cancel for long periods
of time.
This commit is contained in:
Ian Wahbe
2025-05-02 12:31:40 +02:00
committed by GitHub
parent 7ca0d3a6a4
commit 7eb1e3fe50
5 changed files with 25 additions and 18 deletions

View File

@@ -1,5 +1,9 @@
## Unreleased ## Unreleased
### Changed
- Respond to cancel for exec builds. (<https://github.com/pulumi/pulumi-docker-build/pull/522>)
## 0.0.11 (2025-04-11) ## 0.0.11 (2025-04-11)
### Changed ### Changed

View File

@@ -25,6 +25,7 @@ import (
"fmt" "fmt"
"io" "io"
"os" "os"
"os/exec"
"path/filepath" "path/filepath"
"strings" "strings"
@@ -203,7 +204,7 @@ func (c *cli) Close() error {
// execBuild performs a build by os.Exec'ing the docker-buildx binary. // execBuild performs a build by os.Exec'ing the docker-buildx binary.
// Credentials are communicated to docker-buildx via a temporary directory. // Credentials are communicated to docker-buildx via a temporary directory.
// Secrets are communicated via dynamic environment variables. // Secrets are communicated via dynamic environment variables.
func (c *cli) execBuild(b Build) (*client.SolveResponse, error) { func (c *cli) execBuild(ctx context.Context, b Build) (*client.SolveResponse, error) {
// Setup a temporary directory for auth, and clean it up when we're done. // Setup a temporary directory for auth, and clean it up when we're done.
tmp, err := os.MkdirTemp("", "pulumi-docker-") tmp, err := os.MkdirTemp("", "pulumi-docker-")
if err != nil { if err != nil {
@@ -213,7 +214,7 @@ func (c *cli) execBuild(b Build) (*client.SolveResponse, error) {
opts := b.BuildOptions() opts := b.BuildOptions()
builder, err := c.host.builderFor(b) builder, err := c.host.builderFor(ctx, b)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -339,7 +340,7 @@ func (c *cli) execBuild(b Build) (*client.SolveResponse, error) {
} }
// Invoke docker-buildx. // Invoke docker-buildx.
err = c.exec(args, env) err = c.exec(ctx, args, env)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -378,7 +379,7 @@ func (c *cli) execBuild(b Build) (*client.SolveResponse, error) {
// exec invokes a Docker plugin binary. The first argument should be the name // exec invokes a Docker plugin binary. The first argument should be the name
// of the plugin's subcommand, e.g. "buildx". // of the plugin's subcommand, e.g. "buildx".
func (c *cli) exec(args, extraEnv []string) error { func (c *cli) exec(ctx context.Context, args, extraEnv []string) error {
if len(args) == 0 { if len(args) == 0 {
return errors.New("args must be non-empty") return errors.New("args must be non-empty")
} }
@@ -395,16 +396,18 @@ func (c *cli) exec(args, extraEnv []string) error {
defer contract.IgnoreClose(c.w) defer contract.IgnoreClose(c.w)
cmd, err := manager.PluginRunCommand(c, name, root) runCmd, err := manager.PluginRunCommand(c, name, root)
if err != nil { if err != nil {
return err return err
} }
cmd.Args = append([]string{cmd.Args[0]}, args...) // Create a new command that inherits from ctx.
cmd := exec.CommandContext(ctx, //nolint:gosec // We take the first argument and binary from runCmd.
runCmd.Path, append([]string{runCmd.Args[1]}, args...)...,
)
cmd.Stderr = c.Err() cmd.Stderr = c.Err()
cmd.Stdout = c.Out() cmd.Stdout = c.Out()
cmd.Stdin = c.In() cmd.Stdin = c.In()
cmd.Env = append(runCmd.Env, extraEnv...) //nolint:gocritic // We are intentionally assigning from runCmd to cmd
cmd.Env = append(cmd.Env, extraEnv...)
return cmd.Run() return cmd.Run()
} }

View File

@@ -28,12 +28,12 @@ import (
func TestExec(t *testing.T) { func TestExec(t *testing.T) {
t.Parallel() t.Parallel()
h, err := newHost(context.Background(), nil) h, err := newHost(t.Context(), nil)
require.NoError(t, err) require.NoError(t, err)
cli, err := wrap(h) cli, err := wrap(h)
require.NoError(t, err) require.NoError(t, err)
err = cli.exec([]string{"buildx", "version"}, nil) err = cli.exec(t.Context(), []string{"buildx", "version"}, nil)
assert.NoError(t, err) assert.NoError(t, err)
out, err := io.ReadAll(cli.r) out, err := io.ReadAll(cli.r)

View File

@@ -106,10 +106,10 @@ func (c *cli) Build(
defer contract.IgnoreClose(c) defer contract.IgnoreClose(c)
if build.ShouldExec() { if build.ShouldExec() {
return c.execBuild(build) return c.execBuild(ctx, build)
} }
b, err := c.host.builderFor(build) b, err := c.host.builderFor(ctx, build)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@@ -68,7 +68,7 @@ func newHost(ctx context.Context, config *Config) (*host, error) {
// //
// If the build doesn't specify a builder by name, we will iterate through all // If the build doesn't specify a builder by name, we will iterate through all
// available builders until we find one that we can connect to. // available builders until we find one that we can connect to.
func (h *host) builderFor(build Build) (*cachedBuilder, error) { func (h *host) builderFor(ctx context.Context, build Build) (*cachedBuilder, error) {
h.mu.Lock() h.mu.Lock()
defer h.mu.Unlock() defer h.mu.Unlock()
@@ -116,7 +116,7 @@ func (h *host) builderFor(build Build) (*cachedBuilder, error) {
if bb.Err() != nil { if bb.Err() != nil {
continue continue
} }
nodes, err := bb.LoadNodes(context.Background()) nodes, err := bb.LoadNodes(ctx)
if err != nil { if err != nil {
continue continue
} }
@@ -125,7 +125,7 @@ func (h *host) builderFor(build Build) (*cachedBuilder, error) {
if n.Driver == nil { if n.Driver == nil {
continue nextbuilder continue nextbuilder
} }
if _, err := n.Driver.Dial(context.Background()); err != nil { if _, err := n.Driver.Dial(ctx); err != nil {
continue nextbuilder continue nextbuilder
} }
// TODO: Confirm the builder supports the requested platforms. // TODO: Confirm the builder supports the requested platforms.
@@ -139,7 +139,7 @@ func (h *host) builderFor(build Build) (*cachedBuilder, error) {
// If we STILL don't have a builder, create a docker-container instance. // If we STILL don't have a builder, create a docker-container instance.
b, err = builder.Create( b, err = builder.Create(
context.Background(), ctx,
txn, txn,
h.cli, h.cli,
builder.CreateOpts{Driver: "docker-container"}, builder.CreateOpts{Driver: "docker-container"},
@@ -147,7 +147,7 @@ func (h *host) builderFor(build Build) (*cachedBuilder, error) {
if err != nil { if err != nil {
return nil, fmt.Errorf("creating builder: %w", err) return nil, fmt.Errorf("creating builder: %w", err)
} }
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel() defer cancel()
if _, err := b.Boot(ctx); err != nil { if _, err := b.Boot(ctx); err != nil {
return nil, fmt.Errorf("booting builder: %w", err) return nil, fmt.Errorf("booting builder: %w", err)
@@ -157,7 +157,7 @@ func (h *host) builderFor(build Build) (*cachedBuilder, error) {
// Attempt to load nodes in order to determine the builder's driver. Ignore // Attempt to load nodes in order to determine the builder's driver. Ignore
// errors for "exec" builds because it's possible to request builders with // errors for "exec" builds because it's possible to request builders with
// drivers that are unknown to us. // drivers that are unknown to us.
nodes, err := b.LoadNodes(context.Background(), builder.WithData()) nodes, err := b.LoadNodes(ctx, builder.WithData())
if err != nil && !build.ShouldExec() { if err != nil && !build.ShouldExec() {
return nil, fmt.Errorf("loading nodes: %w", err) return nil, fmt.Errorf("loading nodes: %w", err)
} }