Merge pull request #219 from vieux/scale

hide `--detach` for docker < 17.05

cli/command/service/create.go

@@ -124,12 +124,9 @@ func runCreate(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *service
 	fmt.Fprintf(dockerCli.Out(), "%s\n", response.ID)
 
 	if opts.detach {
-		if !flags.Changed("detach") {
+		warnDetachDefault(dockerCli.Err(), apiClient.ClientVersion(), flags, "created")
-			fmt.Fprintln(dockerCli.Err(), "Since --detach=false was not specified, tasks will be created in the background.\n"+
-				"In a future release, --detach=false will become the default.")
-		}
 		return nil
 	}
 
-	return waitOnService(ctx, dockerCli, response.ID, opts)
+	return waitOnService(ctx, dockerCli, response.ID, opts.quiet)
 }

cli/command/service/helpers.go

@@ -1,18 +1,21 @@
 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"
 )
 
 // waitOnService waits for the service to converge. It outputs a progress bar,
 // if appropriate based on the CLI flags.
-func waitOnService(ctx context.Context, dockerCli *command.DockerCli, serviceID string, opts *serviceOptions) error {
+func waitOnService(ctx context.Context, dockerCli *command.DockerCli, serviceID string, quiet bool) error {
 	errChan := make(chan error, 1)
 	pipeReader, pipeWriter := io.Pipe()
 
@@ -20,7 +23,7 @@ func waitOnService(ctx context.Context, dockerCli *command.DockerCli, serviceID
 		errChan <- progress.ServiceProgress(ctx, dockerCli.Client(), serviceID, pipeWriter)
 	}()
 
-	if opts.quiet {
+	if quiet {
 		go io.Copy(ioutil.Discard, pipeReader)
 		return <-errChan
 	}
@@ -31,3 +34,11 @@ func waitOnService(ctx context.Context, dockerCli *command.DockerCli, serviceID
 	}
 	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

@@ -0,0 +1,40 @@
+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

@@ -690,6 +690,11 @@ func buildServiceDefaultFlagMapping() flagDefaults {
 	return defaultFlagValues
 }
 
+func addDetachFlag(flags *pflag.FlagSet, detach *bool) {
+	flags.BoolVarP(detach, flagDetach, "d", true, "Exit immediately instead of waiting for the service to converge")
+	flags.SetAnnotation(flagDetach, "version", []string{"1.29"})
+}
+
 // addServiceFlags adds all flags that are common to both `create` and `update`.
 // Any flags that are not common are added separately in the individual command
 func addServiceFlags(flags *pflag.FlagSet, opts *serviceOptions, defaultFlagValues flagDefaults) {
@@ -700,8 +705,8 @@ func addServiceFlags(flags *pflag.FlagSet, opts *serviceOptions, defaultFlagValu
 		return desc
 	}
 
-	flags.BoolVarP(&opts.detach, "detach", "d", true, "Exit immediately instead of waiting for the service to converge")
+	addDetachFlag(flags, &opts.detach)
-	flags.BoolVarP(&opts.quiet, "quiet", "q", false, "Suppress progress output")
+	flags.BoolVarP(&opts.quiet, flagQuiet, "q", false, "Suppress progress output")
 
 	flags.StringVarP(&opts.workdir, flagWorkdir, "w", "", "Working directory inside the container")
 	flags.StringVarP(&opts.user, flagUser, "u", "", "Username or UID (format: <name|uid>[:<group|gid>])")
@@ -792,6 +797,7 @@ const (
 	flagContainerLabel          = "container-label"
 	flagContainerLabelRemove    = "container-label-rm"
 	flagContainerLabelAdd       = "container-label-add"
+	flagDetach                  = "detach"
 	flagDNS                     = "dns"
 	flagDNSRemove               = "dns-rm"
 	flagDNSAdd                  = "dns-add"
@@ -803,10 +809,6 @@ const (
 	flagDNSSearchAdd            = "dns-search-add"
 	flagEndpointMode            = "endpoint-mode"
 	flagEntrypoint              = "entrypoint"
-	flagHost                    = "host"
-	flagHostAdd                 = "host-add"
-	flagHostRemove              = "host-rm"
-	flagHostname                = "hostname"
 	flagEnv                     = "env"
 	flagEnvFile                 = "env-file"
 	flagEnvRemove               = "env-rm"
@@ -814,6 +816,10 @@ const (
 	flagGroup                   = "group"
 	flagGroupAdd                = "group-add"
 	flagGroupRemove             = "group-rm"
+	flagHost                    = "host"
+	flagHostAdd                 = "host-add"
+	flagHostRemove              = "host-rm"
+	flagHostname                = "hostname"
 	flagLabel                   = "label"
 	flagLabelRemove             = "label-rm"
 	flagLabelAdd                = "label-add"
@@ -830,6 +836,7 @@ const (
 	flagPublish                 = "publish"
 	flagPublishRemove           = "publish-rm"
 	flagPublishAdd              = "publish-add"
+	flagQuiet                   = "quiet"
 	flagReadOnly                = "read-only"
 	flagReplicas                = "replicas"
 	flagReserveCPU              = "reserve-cpu"
@@ -838,6 +845,7 @@ const (
 	flagRestartDelay            = "restart-delay"
 	flagRestartMaxAttempts      = "restart-max-attempts"
 	flagRestartWindow           = "restart-window"
+	flagRollback                = "rollback"
 	flagRollbackDelay           = "rollback-delay"
 	flagRollbackFailureAction   = "rollback-failure-action"
 	flagRollbackMaxFailureRatio = "rollback-max-failure-ratio"

cli/command/service/update.go

@@ -37,8 +37,8 @@ func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command {
 	flags := cmd.Flags()
 	flags.String("image", "", "Service image tag")
 	flags.Var(&ShlexOpt{}, "args", "Service command args")
-	flags.Bool("rollback", false, "Rollback to previous specification")
+	flags.Bool(flagRollback, false, "Rollback to previous specification")
-	flags.SetAnnotation("rollback", "version", []string{"1.25"})
+	flags.SetAnnotation(flagRollback, "version", []string{"1.25"})
 	flags.Bool("force", false, "Force update even if no changes require it")
 	flags.SetAnnotation("force", "version", []string{"1.25"})
 	addServiceFlags(flags, options, nil)
@@ -112,7 +112,7 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, options *serv
 		return err
 	}
 
-	rollback, err := flags.GetBool("rollback")
+	rollback, err := flags.GetBool(flagRollback)
 	if err != nil {
 		return err
 	}
@@ -130,7 +130,7 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, options *serv
 		// Rollback can't be combined with other flags.
 		otherFlagsPassed := false
 		flags.VisitAll(func(f *pflag.Flag) {
-			if f.Name == "rollback" || f.Name == "detach" || f.Name == "quiet" {
+			if f.Name == flagRollback || f.Name == flagDetach || f.Name == flagQuiet {
 				return
 			}
 			if flags.Changed(f.Name) {
@@ -141,7 +141,7 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, options *serv
 			return errors.New("other flags may not be combined with --rollback")
 		}
 
-		if versions.LessThan(dockerCli.Client().ClientVersion(), "1.28") {
+		if versions.LessThan(apiClient.ClientVersion(), "1.28") {
 			clientSideRollback = true
 			spec = service.PreviousSpec
 			if spec == nil {
@@ -217,14 +217,11 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, options *serv
 	fmt.Fprintf(dockerCli.Out(), "%s\n", serviceID)
 
 	if options.detach {
-		if !flags.Changed("detach") {
+		warnDetachDefault(dockerCli.Err(), dockerCli.Client().ClientVersion(), flags, "updated")
-			fmt.Fprintln(dockerCli.Err(), "Since --detach=false was not specified, tasks will be updated in the background.\n"+
-				"In a future release, --detach=false will become the default.")
-		}
 		return nil
 	}
 
-	return waitOnService(ctx, dockerCli, serviceID, options)
+	return waitOnService(ctx, dockerCli, serviceID, options.quiet)
 }
 
 // nolint: gocyclo