diff --git a/CHANGELOG.md b/CHANGELOG.md index 5513e95..53b87fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Remove broken flag `crane start --attach`. This flag was introduced in 3.3.0 but did not work properly, and doesn't add much value anyway. It would make more sense to have this flag on `crane run`, but that is left for another version. + +* Apply `--interactive` to `docker start` only if `--attach` is already passed to it. + ## 3.3.1 (2017-11-20) * Do not add `--interactive` to non-targeted containers, otherwise diff --git a/crane/cli.go b/crane/cli.go index bfeb730..44f3df3 100644 --- a/crane/cli.go +++ b/crane/cli.go @@ -131,10 +131,6 @@ var ( "start", "Start the containers.", ) - startAttachFlag = startCommand.Flag( - "attach", - "Attach to container", - ).Short('a').Bool() startTargetArg = startCommand.Arg("target", "Target of command").String() stopCommand = app.Command( @@ -404,7 +400,7 @@ func runCli() { case startCommand.FullCommand(): commandAction(*startTargetArg, func(uow *UnitOfWork) { - uow.Start(*startAttachFlag) + uow.Start() }, true) case stopCommand.FullCommand(): diff --git a/crane/container.go b/crane/container.go index c422d05..9142136 100644 --- a/crane/container.go +++ b/crane/container.go @@ -22,8 +22,8 @@ type Container interface { Provision(nocache bool) PullImage() Create(cmds []string) - Run(cmds []string, detachFlag bool) - Start(targeted bool, attachFlag bool) + Run(cmds []string, targeted bool, detachFlag bool) + Start(targeted bool) Kill() Stop() Pause() @@ -950,10 +950,8 @@ func (c *container) Create(cmds []string) { // Implemented as create+start as we also need to connect to networks, // and that might fail if we used "docker run" and // have a very short-lived container. -func (c *container) Run(cmds []string, detachFlag bool) { +func (c *container) Run(cmds []string, targeted bool, detachFlag bool) { adHoc := (len(cmds) > 0) - targeted := true - attachFlag := false if !adHoc { c.Rm(true, false) @@ -966,7 +964,7 @@ func (c *container) Run(cmds []string, detachFlag bool) { c.connectWithNetworks(adHoc) - c.start(adHoc, targeted, attachFlag, detachFlag) + c.start(adHoc, targeted, detachFlag) } // Connects container with default network if required, @@ -1375,17 +1373,17 @@ func (c *container) createArgs(cmds []string) []string { } // Start container -func (c *container) Start(targeted bool, attachFlag bool) { +func (c *container) Start(targeted bool) { adHoc := false detachFlag := false if c.Exists() { if !c.Running() { c.startAcceleratedMounts() fmt.Fprintf(c.CommandsOut(), "Starting container %s ...\n", c.ActualName(adHoc)) - c.start(adHoc, targeted, attachFlag, detachFlag) + c.start(adHoc, targeted, detachFlag) } } else { - c.Run([]string{}, detachFlag) + c.Run([]string{}, targeted, detachFlag) } } @@ -1399,34 +1397,33 @@ func (c *container) startAcceleratedMounts() { } } -func (c *container) start(adHoc bool, targeted bool, attachFlag bool, detachFlag bool) { +func (c *container) start(adHoc bool, targeted bool, detachFlag bool) { executeHook(c.Hooks().PreStart(), c.ActualName(adHoc)) args := []string{"start"} - // Attach or detach? - // Precedence: ad-hoc > flags > config > default (targeted) - if attachFlag || adHoc { - args = append(args, "--attach") - } else if !detachFlag { - detach := !targeted - if c.Detach != nil { - detach = *c.Detach - } - if !detach { + // If detach is not configured, it is false by default + configDetach := false + if c.Detach != nil { + configDetach = *c.Detach + } + + // It is only possible to attach to targeted containers + if targeted { + // adHoc always attaches because of --rm + if adHoc || (!detachFlag && !configDetach) { args = append(args, "--attach") + // Interactive - implies attaching! + if c.Stdin_Open || c.Interactive { + args = append(args, "--interactive") + } } } + // DetachKeys if len(c.DetachKeys()) > 0 { args = append(args, "--detach-keys", c.DetachKeys()) } - // Interactive - // Implies attaching to the container, so only applicable - // if the container is being targeted. - if targeted && (c.Stdin_Open || c.Interactive) { - args = append(args, "--interactive") - } args = append(args, c.ActualName(adHoc)) @@ -1486,7 +1483,7 @@ func (c *container) Unpause() { func (c *container) Exec(cmds []string, privileged bool, user string) { name := c.ActualName(false) if !c.Running() { - c.Start(false, false) + c.Start(false) } args := []string{"exec"} if privileged { diff --git a/crane/unit_of_work.go b/crane/unit_of_work.go index 4152872..ffc57a3 100644 --- a/crane/unit_of_work.go +++ b/crane/unit_of_work.go @@ -77,9 +77,9 @@ func (uow *UnitOfWork) Run(cmds []string, detach bool) { uow.prepareRequirements() for _, container := range uow.Containers() { if includes(uow.targeted, container.Name()) { - container.Run(cmds, detach) + container.Run(cmds, true, detach) } else if includes(uow.requireStarted, container.Name()) || !container.Exists() { - container.Start(false, false) + container.Start(false) } } } @@ -89,9 +89,9 @@ func (uow *UnitOfWork) Up(cmds []string, detach bool, noCache bool, parallel int uow.prepareRequirements() for _, container := range uow.Containers() { if includes(uow.targeted, container.Name()) { - container.Run(cmds, detach) + container.Run(cmds, true, detach) } else if includes(uow.requireStarted, container.Name()) || !container.Exists() { - container.Start(false, false) + container.Start(false) } } } @@ -141,13 +141,13 @@ func (uow *UnitOfWork) Pause() { } // Start containers. -func (uow *UnitOfWork) Start(attach bool) { +func (uow *UnitOfWork) Start() { uow.prepareRequirements() for _, container := range uow.Containers() { if includes(uow.targeted, container.Name()) { - container.Start(true, attach) + container.Start(true) } else if includes(uow.requireStarted, container.Name()) || !container.Exists() { - container.Start(false, false) + container.Start(false) } } } @@ -171,7 +171,7 @@ func (uow *UnitOfWork) Exec(cmds []string, privileged bool, user string) { if includes(uow.targeted, container.Name()) { container.Exec(cmds, privileged, user) } else if includes(uow.requireStarted, container.Name()) || !container.Exists() { - container.Start(false, false) + container.Start(false) } } } @@ -190,7 +190,7 @@ func (uow *UnitOfWork) Create(cmds []string) { if includes(uow.targeted, container.Name()) { container.Create(cmds) } else if includes(uow.requireStarted, container.Name()) || !container.Exists() { - container.Start(false, false) + container.Start(false) } } }