From 898e1b605dc54b15d6aee214da6bf2481f66806d Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Wed, 17 Jan 2024 16:44:02 +0000 Subject: [PATCH] signals/utils: always handle received signals The changes in https://github.com/docker/compose/commit/dcbf005fe4fa2b360b8dd013d6dedc4796c37602 fixed the "cancellable context" detection, and made it so that Compose would conditionally set up signal handling when the context was already not cancellable/when the plugin was running through the CLI, as we'd introduced a mechanism into the CLI to signal plugins to exit through a socket instead of handling signals themselves. This had some (not noticed at the time) issues when running through the CLI as, due to sharing a process group id with the parent CLI process, when a user CTRL-Cs the CLI will notify the plugin via the socket but the plugin process itself will also be signalled if attached to the TTY. This impacted some Compose commands that don't set up signal handling - so not `compose up`, but other commands would immediately quit instead of getting some "graceful" cancelled output. We initially attempted to address this "double notification" issue in the CLI by executing plugins under a new pgid so that they wouldn't be signalled, but that posed an issue with Buildx reading from the TTY, (see: https://github.com/moby/moby/issues/47073) so we reverted the process group id changes and ended at a temporary solution in https://github.com/docker/cli/pull/4792 where the CLI will only notify plugins via the socket when they are not already going to be signalled (when attached to a TTY). Due to this, plugins should always set up some signal handling, which this commit implements. Signed-off-by: Laura Brehm --- cmd/compose/compose.go | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index 319bbd9783d..4921450b1fe 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -42,7 +42,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/docker/cli/cli-plugins/plugin" "github.com/docker/compose/v2/cmd/formatter" "github.com/docker/compose/v2/pkg/api" "github.com/docker/compose/v2/pkg/compose" @@ -74,20 +73,17 @@ type CobraCommand func(context.Context, *cobra.Command, []string) error // AdaptCmd adapt a CobraCommand func to cobra library func AdaptCmd(fn CobraCommand) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { - ctx := cmd.Context() - contextString := fmt.Sprintf("%s", ctx) - if !strings.Contains(contextString, ".WithCancel") || plugin.RunningStandalone() { // need to handle cancel - cancellableCtx, cancel := context.WithCancel(cmd.Context()) - ctx = cancellableCtx - s := make(chan os.Signal, 1) - signal.Notify(s, syscall.SIGTERM, syscall.SIGINT) - go func() { - <-s - cancel() - signal.Stop(s) - close(s) - }() - } + ctx, cancel := context.WithCancel(cmd.Context()) + + s := make(chan os.Signal, 1) + signal.Notify(s, syscall.SIGTERM, syscall.SIGINT) + go func() { + <-s + cancel() + signal.Stop(s) + close(s) + }() + err := fn(ctx, cmd, args) var composeErr compose.Error if api.IsErrCanceled(err) || errors.Is(ctx.Err(), context.Canceled) {