Skip to content

Commit

Permalink
hide --detach for docker < 17.05
Browse files Browse the repository at this point in the history
Signed-off-by: Victor Vieux <[email protected]>
  • Loading branch information
vieux committed Jun 27, 2017
1 parent 0133e13 commit db60f25
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 23 deletions.
7 changes: 2 additions & 5 deletions cli/command/service/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
15 changes: 13 additions & 2 deletions cli/command/service/helpers.go
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
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()

go func() {
errChan <- progress.ServiceProgress(ctx, dockerCli.Client(), serviceID, pipeWriter)
}()

if opts.quiet {
if quiet {
go io.Copy(ioutil.Discard, pipeReader)
return <-errChan
}
Expand All @@ -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)
}
}
40 changes: 40 additions & 0 deletions cli/command/service/helpers_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
}
}
20 changes: 14 additions & 6 deletions cli/command/service/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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: <name|uid>[:<group|gid>])")
Expand Down Expand Up @@ -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"
Expand All @@ -803,17 +809,17 @@ 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"
flagEnvAdd = "env-add"
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"
Expand All @@ -830,6 +836,7 @@ const (
flagPublish = "publish"
flagPublishRemove = "publish-rm"
flagPublishAdd = "publish-add"
flagQuiet = "quiet"
flagReadOnly = "read-only"
flagReplicas = "replicas"
flagReserveCPU = "reserve-cpu"
Expand All @@ -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"
Expand Down
17 changes: 7 additions & 10 deletions cli/command/service/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit db60f25

Please sign in to comment.