From 4f738020fda5aae6e508eb86a8967d145da60ea7 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Fri, 5 Jan 2024 12:11:53 +0000 Subject: [PATCH 1/3] deps: remove `appcontext`, use `cmd.Context` Signed-off-by: Laura Brehm --- commands/bake.go | 7 ++----- commands/build.go | 7 ++----- commands/create.go | 8 +++----- commands/diskusage.go | 8 +++----- commands/imagetools/create.go | 7 ++----- commands/imagetools/inspect.go | 9 ++++----- commands/inspect.go | 7 ++----- commands/ls.go | 7 ++----- commands/prune.go | 8 +++----- commands/rm.go | 7 ++----- commands/stop.go | 7 ++----- 11 files changed, 27 insertions(+), 55 deletions(-) diff --git a/commands/bake.go b/commands/bake.go index 5cfa3979..e17a8ebc 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -24,7 +24,6 @@ import ( "github.com/docker/buildx/util/tracing" "github.com/docker/cli/cli/command" "github.com/moby/buildkit/identity" - "github.com/moby/buildkit/util/appcontext" "github.com/moby/buildkit/util/progress/progressui" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -43,9 +42,7 @@ type bakeOptions struct { exportLoad bool } -func runBake(dockerCli command.Cli, targets []string, in bakeOptions, cFlags commonFlags) (err error) { - ctx := appcontext.Context() - +func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in bakeOptions, cFlags commonFlags) (err error) { mp, report, err := metrics.MeterProvider(dockerCli) if err != nil { return err @@ -275,7 +272,7 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { options.builder = rootOpts.builder options.metadataFile = cFlags.metadataFile // Other common flags (noCache, pull and progress) are processed in runBake function. - return runBake(dockerCli, args, options, cFlags) + return runBake(cmd.Context(), dockerCli, args, options, cFlags) }, ValidArgsFunction: completion.BakeTargets(options.files), } diff --git a/commands/build.go b/commands/build.go index a1344684..e142befd 100644 --- a/commands/build.go +++ b/commands/build.go @@ -46,7 +46,6 @@ import ( "github.com/moby/buildkit/frontend/subrequests/outline" "github.com/moby/buildkit/frontend/subrequests/targets" "github.com/moby/buildkit/solver/errdefs" - "github.com/moby/buildkit/util/appcontext" "github.com/moby/buildkit/util/grpcerrors" "github.com/moby/buildkit/util/progress/progressui" "github.com/morikuni/aec" @@ -216,9 +215,7 @@ func (o *buildOptions) toDisplayMode() (progressui.DisplayMode, error) { return progress, nil } -func runBuild(dockerCli command.Cli, options buildOptions) (err error) { - ctx := appcontext.Context() - +func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) (err error) { mp, report, err := metrics.MeterProvider(dockerCli) if err != nil { return err @@ -487,7 +484,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D options.invokeConfig = iConfig } - return runBuild(dockerCli, *options) + return runBuild(cmd.Context(), dockerCli, *options) }, ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { return nil, cobra.ShellCompDirectiveFilterDirs diff --git a/commands/create.go b/commands/create.go index f9f847c2..9362aebb 100644 --- a/commands/create.go +++ b/commands/create.go @@ -2,6 +2,7 @@ package commands import ( "bytes" + "context" "fmt" "github.com/docker/buildx/builder" @@ -11,7 +12,6 @@ import ( "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/moby/buildkit/util/appcontext" "github.com/spf13/cobra" ) @@ -30,9 +30,7 @@ type createOptions struct { // upgrade bool // perform upgrade of the driver } -func runCreate(dockerCli command.Cli, in createOptions, args []string) error { - ctx := appcontext.Context() - +func runCreate(ctx context.Context, dockerCli command.Cli, in createOptions, args []string) error { txn, release, err := storeutil.GetStore(dockerCli) if err != nil { return err @@ -98,7 +96,7 @@ func createCmd(dockerCli command.Cli) *cobra.Command { Short: "Create a new builder instance", Args: cli.RequiresMaxArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runCreate(dockerCli, options, args) + return runCreate(cmd.Context(), dockerCli, options, args) }, ValidArgsFunction: completion.Disable, } diff --git a/commands/diskusage.go b/commands/diskusage.go index dd25e3f3..4460c0fe 100644 --- a/commands/diskusage.go +++ b/commands/diskusage.go @@ -1,6 +1,7 @@ package commands import ( + "context" "fmt" "io" "os" @@ -15,7 +16,6 @@ import ( "github.com/docker/cli/opts" "github.com/docker/go-units" "github.com/moby/buildkit/client" - "github.com/moby/buildkit/util/appcontext" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" ) @@ -26,9 +26,7 @@ type duOptions struct { verbose bool } -func runDiskUsage(dockerCli command.Cli, opts duOptions) error { - ctx := appcontext.Context() - +func runDiskUsage(ctx context.Context, dockerCli command.Cli, opts duOptions) error { pi, err := toBuildkitPruneInfo(opts.filter.Value()) if err != nil { return err @@ -114,7 +112,7 @@ func duCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Args: cli.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder - return runDiskUsage(dockerCli, options) + return runDiskUsage(cmd.Context(), dockerCli, options) }, ValidArgsFunction: completion.Disable, } diff --git a/commands/imagetools/create.go b/commands/imagetools/create.go index 02224113..f05571b0 100644 --- a/commands/imagetools/create.go +++ b/commands/imagetools/create.go @@ -13,7 +13,6 @@ import ( "github.com/docker/buildx/util/imagetools" "github.com/docker/buildx/util/progress" "github.com/docker/cli/cli/command" - "github.com/moby/buildkit/util/appcontext" "github.com/moby/buildkit/util/progress/progressui" "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -32,7 +31,7 @@ type createOptions struct { progress string } -func runCreate(dockerCli command.Cli, in createOptions, args []string) error { +func runCreate(ctx context.Context, dockerCli command.Cli, in createOptions, args []string) error { if len(args) == 0 && len(in.files) == 0 { return errors.Errorf("no sources specified") } @@ -113,8 +112,6 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error { } } - ctx := appcontext.Context() - b, err := builder.New(dockerCli, builder.WithName(in.builder)) if err != nil { return err @@ -274,7 +271,7 @@ func createCmd(dockerCli command.Cli, opts RootOptions) *cobra.Command { Short: "Create a new image based on source images", RunE: func(cmd *cobra.Command, args []string) error { options.builder = *opts.Builder - return runCreate(dockerCli, options, args) + return runCreate(cmd.Context(), dockerCli, options, args) }, ValidArgsFunction: completion.Disable, } diff --git a/commands/imagetools/inspect.go b/commands/imagetools/inspect.go index ad1e6438..5b06f52f 100644 --- a/commands/imagetools/inspect.go +++ b/commands/imagetools/inspect.go @@ -1,13 +1,14 @@ package commands import ( + "context" + "github.com/docker/buildx/builder" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/buildx/util/imagetools" "github.com/docker/cli-docs-tool/annotation" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/moby/buildkit/util/appcontext" "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -18,9 +19,7 @@ type inspectOptions struct { raw bool } -func runInspect(dockerCli command.Cli, in inspectOptions, name string) error { - ctx := appcontext.Context() - +func runInspect(ctx context.Context, dockerCli command.Cli, in inspectOptions, name string) error { if in.format != "" && in.raw { return errors.Errorf("format and raw cannot be used together") } @@ -51,7 +50,7 @@ func inspectCmd(dockerCli command.Cli, rootOpts RootOptions) *cobra.Command { Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { options.builder = *rootOpts.Builder - return runInspect(dockerCli, options, args[0]) + return runInspect(cmd.Context(), dockerCli, options, args[0]) }, ValidArgsFunction: completion.Disable, } diff --git a/commands/inspect.go b/commands/inspect.go index bae2ac6d..e341691c 100644 --- a/commands/inspect.go +++ b/commands/inspect.go @@ -17,7 +17,6 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/debug" "github.com/docker/go-units" - "github.com/moby/buildkit/util/appcontext" "github.com/spf13/cobra" ) @@ -26,9 +25,7 @@ type inspectOptions struct { builder string } -func runInspect(dockerCli command.Cli, in inspectOptions) error { - ctx := appcontext.Context() - +func runInspect(ctx context.Context, dockerCli command.Cli, in inspectOptions) error { b, err := builder.New(dockerCli, builder.WithName(in.builder), builder.WithSkippedValidation(), @@ -150,7 +147,7 @@ func inspectCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { if len(args) > 0 { options.builder = args[0] } - return runInspect(dockerCli, options) + return runInspect(cmd.Context(), dockerCli, options) }, ValidArgsFunction: completion.BuilderNames(dockerCli), } diff --git a/commands/ls.go b/commands/ls.go index 0be95f7a..3b18e42c 100644 --- a/commands/ls.go +++ b/commands/ls.go @@ -17,7 +17,6 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/formatter" - "github.com/moby/buildkit/util/appcontext" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" ) @@ -39,9 +38,7 @@ type lsOptions struct { format string } -func runLs(dockerCli command.Cli, in lsOptions) error { - ctx := appcontext.Context() - +func runLs(ctx context.Context, dockerCli command.Cli, in lsOptions) error { txn, release, err := storeutil.GetStore(dockerCli) if err != nil { return err @@ -103,7 +100,7 @@ func lsCmd(dockerCli command.Cli) *cobra.Command { Short: "List builder instances", Args: cli.ExactArgs(0), RunE: func(cmd *cobra.Command, args []string) error { - return runLs(dockerCli, options) + return runLs(cmd.Context(), dockerCli, options) }, ValidArgsFunction: completion.Disable, } diff --git a/commands/prune.go b/commands/prune.go index 454b04a8..1a2a5220 100644 --- a/commands/prune.go +++ b/commands/prune.go @@ -1,6 +1,7 @@ package commands import ( + "context" "fmt" "os" "strings" @@ -15,7 +16,6 @@ import ( "github.com/docker/docker/api/types/filters" "github.com/docker/go-units" "github.com/moby/buildkit/client" - "github.com/moby/buildkit/util/appcontext" "github.com/pkg/errors" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" @@ -35,9 +35,7 @@ const ( allCacheWarning = `WARNING! This will remove all build cache. Are you sure you want to continue?` ) -func runPrune(dockerCli command.Cli, opts pruneOptions) error { - ctx := appcontext.Context() - +func runPrune(ctx context.Context, dockerCli command.Cli, opts pruneOptions) error { pruneFilters := opts.filter.Value() pruneFilters = command.PruneFilters(dockerCli, pruneFilters) @@ -138,7 +136,7 @@ func pruneCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Args: cli.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder - return runPrune(dockerCli, options) + return runPrune(cmd.Context(), dockerCli, options) }, ValidArgsFunction: completion.Disable, } diff --git a/commands/rm.go b/commands/rm.go index 65293ec6..af6ffd59 100644 --- a/commands/rm.go +++ b/commands/rm.go @@ -10,7 +10,6 @@ import ( "github.com/docker/buildx/store/storeutil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli/command" - "github.com/moby/buildkit/util/appcontext" "github.com/pkg/errors" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" @@ -28,9 +27,7 @@ const ( rmInactiveWarning = `WARNING! This will remove all builders that are not in running state. Are you sure you want to continue?` ) -func runRm(dockerCli command.Cli, in rmOptions) error { - ctx := appcontext.Context() - +func runRm(ctx context.Context, dockerCli command.Cli, in rmOptions) error { if in.allInactive && !in.force && !command.PromptForConfirmation(dockerCli.In(), dockerCli.Out(), rmInactiveWarning) { return nil } @@ -108,7 +105,7 @@ func rmCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { } options.builders = args } - return runRm(dockerCli, options) + return runRm(cmd.Context(), dockerCli, options) }, ValidArgsFunction: completion.BuilderNames(dockerCli), } diff --git a/commands/stop.go b/commands/stop.go index c91b8ad3..b27985af 100644 --- a/commands/stop.go +++ b/commands/stop.go @@ -7,7 +7,6 @@ import ( "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/moby/buildkit/util/appcontext" "github.com/spf13/cobra" ) @@ -15,9 +14,7 @@ type stopOptions struct { builder string } -func runStop(dockerCli command.Cli, in stopOptions) error { - ctx := appcontext.Context() - +func runStop(ctx context.Context, dockerCli command.Cli, in stopOptions) error { b, err := builder.New(dockerCli, builder.WithName(in.builder), builder.WithSkippedValidation(), @@ -45,7 +42,7 @@ func stopCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { if len(args) > 0 { options.builder = args[0] } - return runStop(dockerCli, options) + return runStop(cmd.Context(), dockerCli, options) }, ValidArgsFunction: completion.BuilderNames(dockerCli), } From 650a7af0ae5fe96b37166bfa81ed5cd8cf4df287 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Fri, 5 Jan 2024 13:42:06 +0000 Subject: [PATCH 2/3] cobra/commands: cancel command context on signal See https://github.com/docker/cli/pull/4599 and https://github.com/docker/cli/pull/4769. Since we switched to using the cobra command context instead of `appcontext`, we need to set up the signal handling that was being provided by `appcontext`, as well as configuring the context with the OTEL tracing utilities also used by `appcontext`. This commit introduces `cobrautil.ConfigureContext` which implements the pre-existing signal handling logic from `appcontext` and cancels the command's context when a signal is received, as well as doing the relevant OTEL config. Signed-off-by: Laura Brehm --- commands/bake.go | 5 ++- commands/build.go | 4 +- commands/create.go | 4 +- commands/diskusage.go | 5 ++- commands/imagetools/create.go | 5 ++- commands/imagetools/inspect.go | 5 ++- commands/inspect.go | 5 ++- commands/ls.go | 4 +- commands/prune.go | 5 ++- commands/rm.go | 5 ++- commands/stop.go | 5 ++- util/cobrautil/cobrautil.go | 39 +++++++++++++++++++ util/cobrautil/signals_unix.go | 10 +++++ util/cobrautil/signals_windows.go | 9 +++++ .../buildkit/util/tracing/env/traceenv.go | 4 +- 15 files changed, 90 insertions(+), 24 deletions(-) create mode 100644 util/cobrautil/signals_unix.go create mode 100644 util/cobrautil/signals_windows.go diff --git a/commands/bake.go b/commands/bake.go index e17a8ebc..b07df5fa 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -15,6 +15,7 @@ import ( "github.com/docker/buildx/builder" "github.com/docker/buildx/localstate" "github.com/docker/buildx/util/buildflags" + "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/buildx/util/confutil" "github.com/docker/buildx/util/desktop" @@ -261,7 +262,7 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "bake [OPTIONS] [TARGET...]", Aliases: []string{"f"}, Short: "Build from a file", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { // reset to nil to avoid override is unset if !cmd.Flags().Lookup("no-cache").Changed { cFlags.noCache = nil @@ -273,7 +274,7 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { options.metadataFile = cFlags.metadataFile // Other common flags (noCache, pull and progress) are processed in runBake function. return runBake(cmd.Context(), dockerCli, args, options, cFlags) - }, + }), ValidArgsFunction: completion.BakeTargets(options.files), } diff --git a/commands/build.go b/commands/build.go index e142befd..555fab59 100644 --- a/commands/build.go +++ b/commands/build.go @@ -461,7 +461,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D Aliases: []string{"b"}, Short: "Start a build", Args: cli.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { options.contextPath = args[0] options.builder = rootOpts.builder options.metadataFile = cFlags.metadataFile @@ -485,7 +485,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D } return runBuild(cmd.Context(), dockerCli, *options) - }, + }), ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { return nil, cobra.ShellCompDirectiveFilterDirs }, diff --git a/commands/create.go b/commands/create.go index 9362aebb..a58029ce 100644 --- a/commands/create.go +++ b/commands/create.go @@ -95,9 +95,9 @@ func createCmd(dockerCli command.Cli) *cobra.Command { Use: "create [OPTIONS] [CONTEXT|ENDPOINT]", Short: "Create a new builder instance", Args: cli.RequiresMaxArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { return runCreate(cmd.Context(), dockerCli, options, args) - }, + }), ValidArgsFunction: completion.Disable, } diff --git a/commands/diskusage.go b/commands/diskusage.go index 4460c0fe..2994ae09 100644 --- a/commands/diskusage.go +++ b/commands/diskusage.go @@ -10,6 +10,7 @@ import ( "time" "github.com/docker/buildx/builder" + "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -110,10 +111,10 @@ func duCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "du", Short: "Disk usage", Args: cli.NoArgs, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder return runDiskUsage(cmd.Context(), dockerCli, options) - }, + }), ValidArgsFunction: completion.Disable, } diff --git a/commands/imagetools/create.go b/commands/imagetools/create.go index f05571b0..689e4d58 100644 --- a/commands/imagetools/create.go +++ b/commands/imagetools/create.go @@ -9,6 +9,7 @@ import ( "github.com/distribution/reference" "github.com/docker/buildx/builder" + "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/buildx/util/imagetools" "github.com/docker/buildx/util/progress" @@ -269,10 +270,10 @@ func createCmd(dockerCli command.Cli, opts RootOptions) *cobra.Command { cmd := &cobra.Command{ Use: "create [OPTIONS] [SOURCE] [SOURCE...]", Short: "Create a new image based on source images", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { options.builder = *opts.Builder return runCreate(cmd.Context(), dockerCli, options, args) - }, + }), ValidArgsFunction: completion.Disable, } diff --git a/commands/imagetools/inspect.go b/commands/imagetools/inspect.go index 5b06f52f..e5206b71 100644 --- a/commands/imagetools/inspect.go +++ b/commands/imagetools/inspect.go @@ -4,6 +4,7 @@ import ( "context" "github.com/docker/buildx/builder" + "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/buildx/util/imagetools" "github.com/docker/cli-docs-tool/annotation" @@ -48,10 +49,10 @@ func inspectCmd(dockerCli command.Cli, rootOpts RootOptions) *cobra.Command { Use: "inspect [OPTIONS] NAME", Short: "Show details of an image in the registry", Args: cli.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { options.builder = *rootOpts.Builder return runInspect(cmd.Context(), dockerCli, options, args[0]) - }, + }), ValidArgsFunction: completion.Disable, } diff --git a/commands/inspect.go b/commands/inspect.go index e341691c..90226cdc 100644 --- a/commands/inspect.go +++ b/commands/inspect.go @@ -11,6 +11,7 @@ import ( "github.com/docker/buildx/builder" "github.com/docker/buildx/driver" + "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/buildx/util/platformutil" "github.com/docker/cli/cli" @@ -142,13 +143,13 @@ func inspectCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "inspect [NAME]", Short: "Inspect current builder instance", Args: cli.RequiresMaxArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder if len(args) > 0 { options.builder = args[0] } return runInspect(cmd.Context(), dockerCli, options) - }, + }), ValidArgsFunction: completion.BuilderNames(dockerCli), } diff --git a/commands/ls.go b/commands/ls.go index 3b18e42c..c90c5004 100644 --- a/commands/ls.go +++ b/commands/ls.go @@ -99,9 +99,9 @@ func lsCmd(dockerCli command.Cli) *cobra.Command { Use: "ls", Short: "List builder instances", Args: cli.ExactArgs(0), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { return runLs(cmd.Context(), dockerCli, options) - }, + }), ValidArgsFunction: completion.Disable, } diff --git a/commands/prune.go b/commands/prune.go index 1a2a5220..4dc03e6d 100644 --- a/commands/prune.go +++ b/commands/prune.go @@ -9,6 +9,7 @@ import ( "time" "github.com/docker/buildx/builder" + "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -134,10 +135,10 @@ func pruneCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "prune", Short: "Remove build cache", Args: cli.NoArgs, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder return runPrune(cmd.Context(), dockerCli, options) - }, + }), ValidArgsFunction: completion.Disable, } diff --git a/commands/rm.go b/commands/rm.go index af6ffd59..248fee8e 100644 --- a/commands/rm.go +++ b/commands/rm.go @@ -8,6 +8,7 @@ import ( "github.com/docker/buildx/builder" "github.com/docker/buildx/store" "github.com/docker/buildx/store/storeutil" + "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli/command" "github.com/pkg/errors" @@ -97,7 +98,7 @@ func rmCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { cmd := &cobra.Command{ Use: "rm [OPTIONS] [NAME] [NAME...]", Short: "Remove one or more builder instances", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { options.builders = []string{rootOpts.builder} if len(args) > 0 { if options.allInactive { @@ -106,7 +107,7 @@ func rmCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { options.builders = args } return runRm(cmd.Context(), dockerCli, options) - }, + }), ValidArgsFunction: completion.BuilderNames(dockerCli), } diff --git a/commands/stop.go b/commands/stop.go index b27985af..865ecd93 100644 --- a/commands/stop.go +++ b/commands/stop.go @@ -4,6 +4,7 @@ import ( "context" "github.com/docker/buildx/builder" + "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -37,13 +38,13 @@ func stopCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "stop [NAME]", Short: "Stop builder instance", Args: cli.RequiresMaxArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder if len(args) > 0 { options.builder = args[0] } return runStop(cmd.Context(), dockerCli, options) - }, + }), ValidArgsFunction: completion.BuilderNames(dockerCli), } diff --git a/util/cobrautil/cobrautil.go b/util/cobrautil/cobrautil.go index 71633bb1..97c1aa04 100644 --- a/util/cobrautil/cobrautil.go +++ b/util/cobrautil/cobrautil.go @@ -1,6 +1,13 @@ package cobrautil import ( + "context" + "os" + "os/signal" + + "github.com/moby/buildkit/util/bklog" + detect "github.com/moby/buildkit/util/tracing/env" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -51,3 +58,35 @@ func MarkCommandExperimental(c *cobra.Command) { c.Annotations[annotationExperimentalCLI] = "" c.Short += " (EXPERIMENTAL)" } + +// ConfigureContext sets up signal handling and hooks into the command's +// context so that it will be cancelled when signalled, as well as implementing +// the "hard exit after 3 signals" logic. It also configures OTEL tracing +// for the relevant context. +func ConfigureContext(fn func(*cobra.Command, []string) error) func(cmd *cobra.Command, args []string) error { + return func(cmd *cobra.Command, args []string) error { + ctx := detect.InitContext(cmd.Context()) + cancellableCtx, cancel := context.WithCancelCause(ctx) + ctx = cancellableCtx + + signalLimit := 3 + s := make(chan os.Signal, signalLimit) + signal.Notify(s, interruptSignals...) + go func() { + retries := 0 + for { + <-s + retries++ + err := errors.Errorf("got %d SIGTERM/SIGINTs, forcing shutdown", retries) + cancel(err) + if retries >= signalLimit { + bklog.G(ctx).Errorf(err.Error()) + os.Exit(1) + } + } + }() + + cmd.SetContext(ctx) + return fn(cmd, args) + } +} diff --git a/util/cobrautil/signals_unix.go b/util/cobrautil/signals_unix.go new file mode 100644 index 00000000..8e6899d8 --- /dev/null +++ b/util/cobrautil/signals_unix.go @@ -0,0 +1,10 @@ +//go:build !windows + +package cobrautil + +import ( + "golang.org/x/sys/unix" + "os" +) + +var interruptSignals = []os.Signal{unix.SIGTERM, unix.SIGINT} diff --git a/util/cobrautil/signals_windows.go b/util/cobrautil/signals_windows.go new file mode 100644 index 00000000..bab02ac8 --- /dev/null +++ b/util/cobrautil/signals_windows.go @@ -0,0 +1,9 @@ +//go:build windows + +package cobrautil + +import ( + "os" +) + +var interruptSignals = []os.Signal{os.Interrupt} diff --git a/vendor/github.com/moby/buildkit/util/tracing/env/traceenv.go b/vendor/github.com/moby/buildkit/util/tracing/env/traceenv.go index 0a95619d..711d7cab 100644 --- a/vendor/github.com/moby/buildkit/util/tracing/env/traceenv.go +++ b/vendor/github.com/moby/buildkit/util/tracing/env/traceenv.go @@ -14,10 +14,10 @@ const ( ) func init() { - appcontext.Register(initContext) + appcontext.Register(InitContext) } -func initContext(ctx context.Context) context.Context { +func InitContext(ctx context.Context) context.Context { // open-telemetry/opentelemetry-specification#740 parent := os.Getenv("TRACEPARENT") state := os.Getenv("TRACESTATE") From 147c7135b0a04f4147f59fb7a3cf101be071542b Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Fri, 19 Jan 2024 16:44:30 -0800 Subject: [PATCH 3/3] simplify signal handling for cobra context Signed-off-by: Tonis Tiigi --- commands/bake.go | 5 +-- commands/build.go | 4 +- commands/create.go | 4 +- commands/diskusage.go | 5 +-- commands/imagetools/create.go | 5 +-- commands/imagetools/inspect.go | 5 +-- commands/inspect.go | 5 +-- commands/ls.go | 4 +- commands/prune.go | 5 +-- commands/rm.go | 5 +-- commands/root.go | 14 ++++--- commands/stop.go | 5 +-- util/cobrautil/cobrautil.go | 39 ------------------- util/cobrautil/signals_unix.go | 10 ----- util/cobrautil/signals_windows.go | 9 ----- .../buildkit/util/tracing/env/traceenv.go | 4 +- 16 files changed, 33 insertions(+), 95 deletions(-) delete mode 100644 util/cobrautil/signals_unix.go delete mode 100644 util/cobrautil/signals_windows.go diff --git a/commands/bake.go b/commands/bake.go index b07df5fa..e17a8ebc 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -15,7 +15,6 @@ import ( "github.com/docker/buildx/builder" "github.com/docker/buildx/localstate" "github.com/docker/buildx/util/buildflags" - "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/buildx/util/confutil" "github.com/docker/buildx/util/desktop" @@ -262,7 +261,7 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "bake [OPTIONS] [TARGET...]", Aliases: []string{"f"}, Short: "Build from a file", - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { // reset to nil to avoid override is unset if !cmd.Flags().Lookup("no-cache").Changed { cFlags.noCache = nil @@ -274,7 +273,7 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { options.metadataFile = cFlags.metadataFile // Other common flags (noCache, pull and progress) are processed in runBake function. return runBake(cmd.Context(), dockerCli, args, options, cFlags) - }), + }, ValidArgsFunction: completion.BakeTargets(options.files), } diff --git a/commands/build.go b/commands/build.go index 555fab59..e142befd 100644 --- a/commands/build.go +++ b/commands/build.go @@ -461,7 +461,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D Aliases: []string{"b"}, Short: "Start a build", Args: cli.ExactArgs(1), - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { options.contextPath = args[0] options.builder = rootOpts.builder options.metadataFile = cFlags.metadataFile @@ -485,7 +485,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D } return runBuild(cmd.Context(), dockerCli, *options) - }), + }, ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { return nil, cobra.ShellCompDirectiveFilterDirs }, diff --git a/commands/create.go b/commands/create.go index a58029ce..9362aebb 100644 --- a/commands/create.go +++ b/commands/create.go @@ -95,9 +95,9 @@ func createCmd(dockerCli command.Cli) *cobra.Command { Use: "create [OPTIONS] [CONTEXT|ENDPOINT]", Short: "Create a new builder instance", Args: cli.RequiresMaxArgs(1), - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { return runCreate(cmd.Context(), dockerCli, options, args) - }), + }, ValidArgsFunction: completion.Disable, } diff --git a/commands/diskusage.go b/commands/diskusage.go index 2994ae09..4460c0fe 100644 --- a/commands/diskusage.go +++ b/commands/diskusage.go @@ -10,7 +10,6 @@ import ( "time" "github.com/docker/buildx/builder" - "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -111,10 +110,10 @@ func duCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "du", Short: "Disk usage", Args: cli.NoArgs, - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder return runDiskUsage(cmd.Context(), dockerCli, options) - }), + }, ValidArgsFunction: completion.Disable, } diff --git a/commands/imagetools/create.go b/commands/imagetools/create.go index 689e4d58..f05571b0 100644 --- a/commands/imagetools/create.go +++ b/commands/imagetools/create.go @@ -9,7 +9,6 @@ import ( "github.com/distribution/reference" "github.com/docker/buildx/builder" - "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/buildx/util/imagetools" "github.com/docker/buildx/util/progress" @@ -270,10 +269,10 @@ func createCmd(dockerCli command.Cli, opts RootOptions) *cobra.Command { cmd := &cobra.Command{ Use: "create [OPTIONS] [SOURCE] [SOURCE...]", Short: "Create a new image based on source images", - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { options.builder = *opts.Builder return runCreate(cmd.Context(), dockerCli, options, args) - }), + }, ValidArgsFunction: completion.Disable, } diff --git a/commands/imagetools/inspect.go b/commands/imagetools/inspect.go index e5206b71..5b06f52f 100644 --- a/commands/imagetools/inspect.go +++ b/commands/imagetools/inspect.go @@ -4,7 +4,6 @@ import ( "context" "github.com/docker/buildx/builder" - "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/buildx/util/imagetools" "github.com/docker/cli-docs-tool/annotation" @@ -49,10 +48,10 @@ func inspectCmd(dockerCli command.Cli, rootOpts RootOptions) *cobra.Command { Use: "inspect [OPTIONS] NAME", Short: "Show details of an image in the registry", Args: cli.ExactArgs(1), - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { options.builder = *rootOpts.Builder return runInspect(cmd.Context(), dockerCli, options, args[0]) - }), + }, ValidArgsFunction: completion.Disable, } diff --git a/commands/inspect.go b/commands/inspect.go index 90226cdc..e341691c 100644 --- a/commands/inspect.go +++ b/commands/inspect.go @@ -11,7 +11,6 @@ import ( "github.com/docker/buildx/builder" "github.com/docker/buildx/driver" - "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/buildx/util/platformutil" "github.com/docker/cli/cli" @@ -143,13 +142,13 @@ func inspectCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "inspect [NAME]", Short: "Inspect current builder instance", Args: cli.RequiresMaxArgs(1), - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder if len(args) > 0 { options.builder = args[0] } return runInspect(cmd.Context(), dockerCli, options) - }), + }, ValidArgsFunction: completion.BuilderNames(dockerCli), } diff --git a/commands/ls.go b/commands/ls.go index c90c5004..3b18e42c 100644 --- a/commands/ls.go +++ b/commands/ls.go @@ -99,9 +99,9 @@ func lsCmd(dockerCli command.Cli) *cobra.Command { Use: "ls", Short: "List builder instances", Args: cli.ExactArgs(0), - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { return runLs(cmd.Context(), dockerCli, options) - }), + }, ValidArgsFunction: completion.Disable, } diff --git a/commands/prune.go b/commands/prune.go index 4dc03e6d..1a2a5220 100644 --- a/commands/prune.go +++ b/commands/prune.go @@ -9,7 +9,6 @@ import ( "time" "github.com/docker/buildx/builder" - "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -135,10 +134,10 @@ func pruneCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "prune", Short: "Remove build cache", Args: cli.NoArgs, - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder return runPrune(cmd.Context(), dockerCli, options) - }), + }, ValidArgsFunction: completion.Disable, } diff --git a/commands/rm.go b/commands/rm.go index 248fee8e..af6ffd59 100644 --- a/commands/rm.go +++ b/commands/rm.go @@ -8,7 +8,6 @@ import ( "github.com/docker/buildx/builder" "github.com/docker/buildx/store" "github.com/docker/buildx/store/storeutil" - "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli/command" "github.com/pkg/errors" @@ -98,7 +97,7 @@ func rmCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { cmd := &cobra.Command{ Use: "rm [OPTIONS] [NAME] [NAME...]", Short: "Remove one or more builder instances", - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { options.builders = []string{rootOpts.builder} if len(args) > 0 { if options.allInactive { @@ -107,7 +106,7 @@ func rmCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { options.builders = args } return runRm(cmd.Context(), dockerCli, options) - }), + }, ValidArgsFunction: completion.BuilderNames(dockerCli), } diff --git a/commands/root.go b/commands/root.go index 780b689d..1c862489 100644 --- a/commands/root.go +++ b/commands/root.go @@ -13,6 +13,7 @@ import ( "github.com/docker/cli/cli-plugins/plugin" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/debug" + "github.com/moby/buildkit/util/appcontext" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -29,12 +30,15 @@ func NewRootCmd(name string, isPlugin bool, dockerCli command.Cli) *cobra.Comman CompletionOptions: cobra.CompletionOptions{ HiddenDefaultCmd: true, }, - } - if isPlugin { - cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + cmd.SetContext(appcontext.Context()) + if !isPlugin { + return nil + } return plugin.PersistentPreRunE(cmd, args) - } - } else { + }, + } + if !isPlugin { // match plugin behavior for standalone mode // https://github.com/docker/cli/blob/6c9eb708fa6d17765d71965f90e1c59cea686ee9/cli-plugins/plugin/plugin.go#L117-L127 cmd.SilenceUsage = true diff --git a/commands/stop.go b/commands/stop.go index 865ecd93..b27985af 100644 --- a/commands/stop.go +++ b/commands/stop.go @@ -4,7 +4,6 @@ import ( "context" "github.com/docker/buildx/builder" - "github.com/docker/buildx/util/cobrautil" "github.com/docker/buildx/util/cobrautil/completion" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -38,13 +37,13 @@ func stopCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command { Use: "stop [NAME]", Short: "Stop builder instance", Args: cli.RequiresMaxArgs(1), - RunE: cobrautil.ConfigureContext(func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { options.builder = rootOpts.builder if len(args) > 0 { options.builder = args[0] } return runStop(cmd.Context(), dockerCli, options) - }), + }, ValidArgsFunction: completion.BuilderNames(dockerCli), } diff --git a/util/cobrautil/cobrautil.go b/util/cobrautil/cobrautil.go index 97c1aa04..71633bb1 100644 --- a/util/cobrautil/cobrautil.go +++ b/util/cobrautil/cobrautil.go @@ -1,13 +1,6 @@ package cobrautil import ( - "context" - "os" - "os/signal" - - "github.com/moby/buildkit/util/bklog" - detect "github.com/moby/buildkit/util/tracing/env" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -58,35 +51,3 @@ func MarkCommandExperimental(c *cobra.Command) { c.Annotations[annotationExperimentalCLI] = "" c.Short += " (EXPERIMENTAL)" } - -// ConfigureContext sets up signal handling and hooks into the command's -// context so that it will be cancelled when signalled, as well as implementing -// the "hard exit after 3 signals" logic. It also configures OTEL tracing -// for the relevant context. -func ConfigureContext(fn func(*cobra.Command, []string) error) func(cmd *cobra.Command, args []string) error { - return func(cmd *cobra.Command, args []string) error { - ctx := detect.InitContext(cmd.Context()) - cancellableCtx, cancel := context.WithCancelCause(ctx) - ctx = cancellableCtx - - signalLimit := 3 - s := make(chan os.Signal, signalLimit) - signal.Notify(s, interruptSignals...) - go func() { - retries := 0 - for { - <-s - retries++ - err := errors.Errorf("got %d SIGTERM/SIGINTs, forcing shutdown", retries) - cancel(err) - if retries >= signalLimit { - bklog.G(ctx).Errorf(err.Error()) - os.Exit(1) - } - } - }() - - cmd.SetContext(ctx) - return fn(cmd, args) - } -} diff --git a/util/cobrautil/signals_unix.go b/util/cobrautil/signals_unix.go deleted file mode 100644 index 8e6899d8..00000000 --- a/util/cobrautil/signals_unix.go +++ /dev/null @@ -1,10 +0,0 @@ -//go:build !windows - -package cobrautil - -import ( - "golang.org/x/sys/unix" - "os" -) - -var interruptSignals = []os.Signal{unix.SIGTERM, unix.SIGINT} diff --git a/util/cobrautil/signals_windows.go b/util/cobrautil/signals_windows.go deleted file mode 100644 index bab02ac8..00000000 --- a/util/cobrautil/signals_windows.go +++ /dev/null @@ -1,9 +0,0 @@ -//go:build windows - -package cobrautil - -import ( - "os" -) - -var interruptSignals = []os.Signal{os.Interrupt} diff --git a/vendor/github.com/moby/buildkit/util/tracing/env/traceenv.go b/vendor/github.com/moby/buildkit/util/tracing/env/traceenv.go index 711d7cab..0a95619d 100644 --- a/vendor/github.com/moby/buildkit/util/tracing/env/traceenv.go +++ b/vendor/github.com/moby/buildkit/util/tracing/env/traceenv.go @@ -14,10 +14,10 @@ const ( ) func init() { - appcontext.Register(InitContext) + appcontext.Register(initContext) } -func InitContext(ctx context.Context) context.Context { +func initContext(ctx context.Context) context.Context { // open-telemetry/opentelemetry-specification#740 parent := os.Getenv("TRACEPARENT") state := os.Getenv("TRACESTATE")