diff --git a/cli/command/service/create.go b/cli/command/service/create.go index ef73ec19ed..e4b42a45ce 100644 --- a/cli/command/service/create.go +++ b/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") { - 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.") - } + warnDetachDefault(dockerCli.Err(), apiClient.ClientVersion(), flags, "created") return nil } - return waitOnService(ctx, dockerCli, response.ID, opts) + return waitOnService(ctx, dockerCli, response.ID, opts.quiet) } diff --git a/cli/command/service/helpers.go b/cli/command/service/helpers.go index 8bc745dcef..8fb82c7cca 100644 --- a/cli/command/service/helpers.go +++ b/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) + } +} diff --git a/cli/command/service/helpers_test.go b/cli/command/service/helpers_test.go new file mode 100644 index 0000000000..6c63209b6b --- /dev/null +++ b/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") + } + } +} diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index 4c1425c42c..f2381881d7 100644 --- a/cli/command/service/opts.go +++ b/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") - flags.BoolVarP(&opts.quiet, "quiet", "q", false, "Suppress progress output") + addDetachFlag(flags, &opts.detach) + 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: [:])") @@ -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" diff --git a/cli/command/service/update.go b/cli/command/service/update.go index a80bf01ef1..dba05a1b1e 100644 --- a/cli/command/service/update.go +++ b/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.SetAnnotation("rollback", "version", []string{"1.25"}) + flags.Bool(flagRollback, false, "Rollback to previous specification") + 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") { - 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.") - } + warnDetachDefault(dockerCli.Err(), dockerCli.Client().ClientVersion(), flags, "updated") return nil } - return waitOnService(ctx, dockerCli, serviceID, options) + return waitOnService(ctx, dockerCli, serviceID, options.quiet) } // nolint: gocyclo