From ce5a0c656f140f577a8a304298dba336427dcbb5 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Wed, 20 Dec 2023 13:06:15 +0000 Subject: [PATCH 1/2] Fix cancellable context detection in `AdaptCmd` `AdaptCmd` was previously checking for a `.WithCancel` suffix on context strings, however it's possible for a context to be cancellable without ending in that suffix, such as when `context.WithValue` was called after `WithContext`, e.g.: ```go context.Background.WithCancel.WithValue(type trace.traceContextKeyType, val ).WithValue(type api.DryRunKey, val ) ``` Signed-off-by: Laura Brehm --- cmd/compose/compose.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index d2f8e98e9..72d8effc7 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -42,6 +42,7 @@ 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" @@ -75,7 +76,7 @@ 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.HasSuffix(contextString, ".WithCancel") { // need to handle cancel + 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) @@ -277,7 +278,12 @@ const PluginName = "compose" // RunningAsStandalone detects when running as a standalone program func RunningAsStandalone() bool { - return len(os.Args) < 2 || os.Args[1] != manager.MetadataSubcommandName && os.Args[1] != PluginName + println("check running as standalone") + standalone := len(os.Args) < 2 || os.Args[1] != manager.MetadataSubcommandName && os.Args[1] != PluginName + fmt.Fprintf(os.Stderr, "%v+\n", os.Args) + println("len os.args", len(os.Args)) + println("STANDALONE:", standalone) + return standalone } // RootCommand returns the compose command with its child commands From dcbf005fe4fa2b360b8dd013d6dedc4796c37602 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Wed, 20 Dec 2023 17:45:51 +0000 Subject: [PATCH 2/2] up: gracefully teardown when command ctx cancelled Previously, if a long-lived plugin process (such as an execution of `compose up`) was running and then detached from a terminal, signalling the parent CLI process to exit would leave the plugin process behind. To address this, changes were introduced on the CLI side (see: https://github.com/docker/cli/pull/4599) to enable the CLI to notify a running plugin process that it should exit. This makes it so that, when the parent CLI process is going to exit, the command context of the plugin command being executed is cancelled. This commit takes advantage of these changes by tapping into the command context's done channel and using it to teardown on an up. Signed-off-by: Laura Brehm --- cmd/compose/compose.go | 7 +------ pkg/compose/up.go | 33 ++++++++++++++++++++------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index 72d8effc7..ea076c2ef 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -278,12 +278,7 @@ const PluginName = "compose" // RunningAsStandalone detects when running as a standalone program func RunningAsStandalone() bool { - println("check running as standalone") - standalone := len(os.Args) < 2 || os.Args[1] != manager.MetadataSubcommandName && os.Args[1] != PluginName - fmt.Fprintf(os.Stderr, "%v+\n", os.Args) - println("len os.args", len(os.Args)) - println("STANDALONE:", standalone) - return standalone + return len(os.Args) < 2 || os.Args[1] != manager.MetadataSubcommandName && os.Args[1] != PluginName } // RootCommand returns the compose command with its child commands diff --git a/pkg/compose/up.go b/pkg/compose/up.go index fb54a5965..8236adff0 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -31,7 +31,7 @@ import ( "github.com/hashicorp/go-multierror" ) -func (s *composeService) Up(ctx context.Context, project *types.Project, options api.UpOptions) error { +func (s *composeService) Up(ctx context.Context, project *types.Project, options api.UpOptions) error { //nolint:gocyclo err := progress.Run(ctx, tracing.SpanWrapFunc("project/up", tracing.ProjectOptions(project), func(ctx context.Context) error { err := s.create(ctx, project, options.Create) if err != nil { @@ -69,24 +69,31 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options doneCh := make(chan bool) eg.Go(func() error { first := true + gracefulTeardown := func() { + printer.Cancel() + fmt.Fprintln(s.stdinfo(), "Gracefully stopping... (press Ctrl+C again to force)") + eg.Go(func() error { + err := s.Stop(context.Background(), project.Name, api.StopOptions{ + Services: options.Create.Services, + Project: project, + }) + isTerminated = true + close(doneCh) + return err + }) + first = false + } for { select { case <-doneCh: return nil + case <-ctx.Done(): + if first { + gracefulTeardown() + } case <-signalChan: if first { - printer.Cancel() - fmt.Fprintln(s.stdinfo(), "Gracefully stopping... (press Ctrl+C again to force)") - eg.Go(func() error { - err := s.Stop(context.Background(), project.Name, api.StopOptions{ - Services: options.Create.Services, - Project: project, - }) - isTerminated = true - close(doneCh) - return err - }) - first = false + gracefulTeardown() } else { eg.Go(func() error { return s.Kill(context.Background(), project.Name, api.KillOptions{