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 db60f25561), 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 <github@gone.nl>

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
 	}
 

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)
-	}
-}

cli/command/service/helpers_test.go

@@ -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")
-		}
-	}
-}

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"})
 }
 

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
 	}
 

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))

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
 	}
 

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