Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hide --detach for docker < 17.05 #219

Merged
merged 1 commit into from
Jun 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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