From 0c27355f7b07943425d360d1345c625f0e60b8cf Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 12 Sep 2017 14:35:19 +0200 Subject: [PATCH] Use non-detached mode as default for service commands Commit 330a0035334871d92207b583c1c36d52a244753f added a `--detach=false` option to various service-related commands, with the intent to make this the default in a future version (17.09). This patch changes the default to use "interactive" (non-detached), allowing users to override this by setting the `--detach` option. To prevent problems when connecting to older daemon versions (17.05 and below, see commit db60f255617fd90cb093813dcdfe7eec840eeff8), the detach option is ignored for those versions, and detach is always true. Before this change, a warning was printed to announce the upcoming default: $ docker service create nginx:alpine saxiyn3pe559d753730zr0xer Since --detach=false was not specified, tasks will be created in the background. In a future release, --detach=false will become the default. After this change, no warning is printed, but `--detach` is disabled; $ docker service create nginx:alpine y9jujwzozi0hwgj5yaadzliq6 overall progress: 1 out of 1 tasks 1/1: running [==================================================>] verify: Service converged Setting the `--detach` flag makes the cli use the pre-17.06 behavior: $ docker service create --detach nginx:alpine 280hjnzy0wzje5o56gr22a46n Running against a 17.03 daemon, without specifying the `--detach` flag; $ docker service create nginx:alpine kqheg7ogj0kszoa34g4p73i8q Signed-off-by: Sebastiaan van Stijn --- cli/command/service/create.go | 3 +-- cli/command/service/helpers.go | 11 -------- cli/command/service/helpers_test.go | 40 ----------------------------- cli/command/service/opts.go | 2 +- cli/command/service/rollback.go | 9 +++---- cli/command/service/scale.go | 10 +++----- cli/command/service/update.go | 3 +-- docs/deprecated.md | 2 +- 8 files changed, 12 insertions(+), 68 deletions(-) delete mode 100644 cli/command/service/helpers_test.go diff --git a/cli/command/service/create.go b/cli/command/service/create.go index ecf7a37e281d..84944fd23c82 100644 --- a/cli/command/service/create.go +++ b/cli/command/service/create.go @@ -123,8 +123,7 @@ func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, opts *serviceOptions fmt.Fprintf(dockerCli.Out(), "%s\n", response.ID) - if opts.detach { - warnDetachDefault(dockerCli.Err(), apiClient.ClientVersion(), flags, "created") + if opts.detach || versions.LessThan(apiClient.ClientVersion(), "1.29") { return nil } diff --git a/cli/command/service/helpers.go b/cli/command/service/helpers.go index de6878c17bd4..f328c7059aff 100644 --- a/cli/command/service/helpers.go +++ b/cli/command/service/helpers.go @@ -1,15 +1,12 @@ package service import ( - "fmt" "io" "io/ioutil" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/service/progress" - "github.com/docker/docker/api/types/versions" "github.com/docker/docker/pkg/jsonmessage" - "github.com/spf13/pflag" "golang.org/x/net/context" ) @@ -34,11 +31,3 @@ func waitOnService(ctx context.Context, dockerCli command.Cli, serviceID string, } return err } - -// warnDetachDefault warns about the --detach flag future change if it's supported. -func warnDetachDefault(err io.Writer, clientVersion string, flags *pflag.FlagSet, msg string) { - if !flags.Changed("detach") && versions.GreaterThanOrEqualTo(clientVersion, "1.29") { - fmt.Fprintf(err, "Since --detach=false was not specified, tasks will be %s in the background.\n"+ - "In a future release, --detach=false will become the default.\n", msg) - } -} diff --git a/cli/command/service/helpers_test.go b/cli/command/service/helpers_test.go deleted file mode 100644 index 6c63209b6b37..000000000000 --- a/cli/command/service/helpers_test.go +++ /dev/null @@ -1,40 +0,0 @@ -package service - -import ( - "bytes" - "testing" - - "github.com/spf13/pflag" - "github.com/stretchr/testify/assert" -) - -func TestWarnDetachDefault(t *testing.T) { - var detach bool - flags := pflag.NewFlagSet("test", pflag.ContinueOnError) - addDetachFlag(flags, &detach) - - var tests = []struct { - detach bool - version string - - expectWarning bool - }{ - {true, "1.28", false}, - {true, "1.29", false}, - {false, "1.28", false}, - {false, "1.29", true}, - } - - for _, test := range tests { - out := new(bytes.Buffer) - flags.Lookup(flagDetach).Changed = test.detach - - warnDetachDefault(out, test.version, flags, "") - - if test.expectWarning { - assert.NotEmpty(t, out.String(), "expected warning") - } else { - assert.Empty(t, out.String(), "expected no warning") - } - } -} diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index fc94c7d913a1..37369f595b34 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -691,7 +691,7 @@ func buildServiceDefaultFlagMapping() flagDefaults { } func addDetachFlag(flags *pflag.FlagSet, detach *bool) { - flags.BoolVarP(detach, flagDetach, "d", true, "Exit immediately instead of waiting for the service to converge") + flags.BoolVarP(detach, flagDetach, "d", false, "Exit immediately instead of waiting for the service to converge") flags.SetAnnotation(flagDetach, "version", []string{"1.29"}) } diff --git a/cli/command/service/rollback.go b/cli/command/service/rollback.go index 5cef656255b4..375e6d510a21 100644 --- a/cli/command/service/rollback.go +++ b/cli/command/service/rollback.go @@ -7,8 +7,8 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/versions" "github.com/spf13/cobra" - "github.com/spf13/pflag" ) func newRollbackCommand(dockerCli command.Cli) *cobra.Command { @@ -19,7 +19,7 @@ func newRollbackCommand(dockerCli command.Cli) *cobra.Command { Short: "Revert changes to a service's configuration", Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runRollback(dockerCli, cmd.Flags(), options, args[0]) + return runRollback(dockerCli, options, args[0]) }, Tags: map[string]string{"version": "1.31"}, } @@ -31,7 +31,7 @@ func newRollbackCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runRollback(dockerCli command.Cli, flags *pflag.FlagSet, options *serviceOptions, serviceID string) error { +func runRollback(dockerCli command.Cli, options *serviceOptions, serviceID string) error { apiClient := dockerCli.Client() ctx := context.Background() @@ -56,8 +56,7 @@ func runRollback(dockerCli command.Cli, flags *pflag.FlagSet, options *serviceOp fmt.Fprintf(dockerCli.Out(), "%s\n", serviceID) - if options.detach { - warnDetachDefault(dockerCli.Err(), apiClient.ClientVersion(), flags, "rolled back") + if options.detach || versions.LessThan(apiClient.ClientVersion(), "1.29") { return nil } diff --git a/cli/command/service/scale.go b/cli/command/service/scale.go index 4910d4fa503e..d38b01b4b51d 100644 --- a/cli/command/service/scale.go +++ b/cli/command/service/scale.go @@ -10,9 +10,9 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/versions" "github.com/pkg/errors" "github.com/spf13/cobra" - "github.com/spf13/pflag" ) type scaleOptions struct { @@ -27,7 +27,7 @@ func newScaleCommand(dockerCli command.Cli) *cobra.Command { Short: "Scale one or multiple replicated services", Args: scaleArgs, RunE: func(cmd *cobra.Command, args []string) error { - return runScale(dockerCli, cmd.Flags(), options, args) + return runScale(dockerCli, options, args) }, } @@ -54,7 +54,7 @@ func scaleArgs(cmd *cobra.Command, args []string) error { return nil } -func runScale(dockerCli command.Cli, flags *pflag.FlagSet, options *scaleOptions, args []string) error { +func runScale(dockerCli command.Cli, options *scaleOptions, args []string) error { var errs []string var serviceIDs []string ctx := context.Background() @@ -79,9 +79,7 @@ func runScale(dockerCli command.Cli, flags *pflag.FlagSet, options *scaleOptions } if len(serviceIDs) > 0 { - if options.detach { - warnDetachDefault(dockerCli.Err(), dockerCli.Client().ClientVersion(), flags, "scaled") - } else { + if !options.detach && versions.GreaterThanOrEqualTo(dockerCli.Client().ClientVersion(), "1.29") { for _, serviceID := range serviceIDs { if err := waitOnService(ctx, dockerCli, serviceID, false); err != nil { errs = append(errs, fmt.Sprintf("%s: %v", serviceID, err)) diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 02a940db9dcb..6ee0dfc742c0 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -216,8 +216,7 @@ func runUpdate(dockerCli command.Cli, flags *pflag.FlagSet, options *serviceOpti fmt.Fprintf(dockerCli.Out(), "%s\n", serviceID) - if options.detach { - warnDetachDefault(dockerCli.Err(), dockerCli.Client().ClientVersion(), flags, "updated") + if options.detach || versions.LessThan(apiClient.ClientVersion(), "1.29") { return nil } diff --git a/docs/deprecated.md b/docs/deprecated.md index e6714fee135a..60187c5eab4f 100644 --- a/docs/deprecated.md +++ b/docs/deprecated.md @@ -24,7 +24,7 @@ see [Feature Deprecation Policy](https://docs.docker.com/engine/#feature-depreca **Deprecated In Release: v17.05.0** -**Disabled by default in release: v17.09** +**Disabled by default in release: [v17.09](https://github.com/docker/docker-ce/releases/tag/v17.09.0-ce)** Docker 17.05.0 added an optional `--detach=false` option to make the `docker service create` and `docker service update` work synchronously. This